Skip to content

Conversation

deejgregor
Copy link
Contributor

@deejgregor deejgregor commented Jul 3, 2025

What Does This Do

Adds error priorities for dynamic instrumentation and manual instrumentation, similar to what exists for priorities for resource name today.

It also sets the priority for manual instrumentation from otel spans for both resource name and error.

Motivation

Reliably override error status set by automatic instrumentation from dynamic instrumentation and manual instrumentation.

Additional Notes

No tests have been added yet. If the change generally seems good, I'm happy to write tests.

Contributor Checklist

Jira ticket: [PROJ-IDENT]

@deejgregor deejgregor requested review from a team as code owners July 3, 2025 21:12
@deejgregor deejgregor requested a review from amarziali July 3, 2025 21:12
@amarziali
Copy link
Contributor

amarziali commented Jul 8, 2025

👋 @deejgregor Thanks for having opened this pull request. Having one more level for manual instrumentation makes sense. I think there is no need to create an additional one for the TagInterceptor, since the setError here uses the default (That will be lower of the one created for manual instrumentation).
Also, if you can, would it be possible to add a test for this feature?
Thanks again for the contribution
Andrea

@amarziali amarziali added type: enhancement Enhancements and improvements comp: core Tracer core labels Jul 8, 2025
@@ -217,7 +217,7 @@ private boolean interceptDbStatement(DDSpanContext span, Object value) {
}

private boolean interceptError(DDSpanContext span, Object value) {
span.setErrorFlag(asBoolean(value), ErrorPriorities.DEFAULT);
span.setErrorFlag(asBoolean(value), ErrorPriorities.TAG_INTERCEPTOR);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would still use the default here. The ones using the interceptor, are setting a tag to infer the error flag.Both are falling hence into the manual instrumentation use case

@PerfectSlayer PerfectSlayer added the tag: community Community contribution label Jul 9, 2025
@deejgregor deejgregor force-pushed the feature-more-error-priorities branch from d616ce3 to f573ade Compare July 25, 2025 06:36
@cecile75
Copy link
Contributor

👋 @deejgregor ,
Do you agree with @amarziali 's comment about the simplification: removing the ErrorPriorities.TAG_INTERCEPTOR value and using the default priority in the TagInterceptor.java file?
Could you add some tests for this feature? Please let us know if you can't.
Thank you,
Cécile

@cecile75
Copy link
Contributor

Hi @deejgregor ,
I've cherry-picked your commits and added them to #9387.
This change should be included in the next release 1.53.0.
Thank you again for this suggestion and PR!
Cécile

@cecile75
Copy link
Contributor

Closing this PR. The modifications have been included in the release 1.53.0. Thank you again for this idea and collaboration @deejgregor !

@cecile75 cecile75 closed this Sep 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp: core Tracer core tag: community Community contribution type: enhancement Enhancements and improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants