-
Notifications
You must be signed in to change notification settings - Fork 315
Resilience4j 2+ #9525
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Resilience4j 2+ #9525
Conversation
Clean up CircuitBreakerInstrumentation
…ion. Fix muzzle check by excluding helper classes referencing an instrumented interface. Extracted Resilience4jInstrumentation.
Minimize number of necessary wrapper. Get rid of the muzzle problem caused by a wrapper extending an instrumented interface.
…or span finishing
Add sync retry test.
Pass the context holder at creation time. TODO: Implement this for all types of combinators and eliminate reliance on the active span.
Pass the context holder at creation time. TODO: Implement this for tracedCompletionStage
Pass the context holder at creation time.
…tead of trying to build a common context holder at a decorator construction time. This should simplify the solution and make it less prone to missing instrumentation and easier to apply to other integration like reactor, javarx, etc.
|
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
.../src/main/java/datadog/trace/instrumentation/resilience4j/CircuitBreakerInstrumentation.java
Outdated
Show resolved
Hide resolved
cdb0f47 to
5b2e1ab
Compare
|
@amarziali Thanks for your suggestions! I've made the necessary improvements. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for having improved the library support with the resilience4j instrumentations. The changes looks good. I left few comments especially around strings that could be statically declared as constants
...4j-2.0/src/main/java/datadog/trace/instrumentation/resilience4j/CircuitBreakerDecorator.java
Show resolved
Hide resolved
...ilience4j-2.0/src/main/java/datadog/trace/instrumentation/resilience4j/Resilience4jSpan.java
Outdated
Show resolved
Hide resolved
...esilience4j-2.0/src/main/java/datadog/trace/instrumentation/resilience4j/RetryDecorator.java
Show resolved
Hide resolved
|
|
||
| @Override | ||
| protected CharSequence spanType() { | ||
| return null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
other instrumentations create a span type. why this is not doing the same?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm leaving it null because I don't see a suitable candidate among InternalSpanTypes. This is the default for many instrumentations and is treated as a custom span type. Do you think I need to be more specific in this case? I'm asking because I'm not sure how it's being used. If I'm not mistaken, OTel doesn't seem to have this concept.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can create your own span type if needed. It depends than if that tagging is used internally for something. i.e. you can use it for a particular display in the future. If not you can leave blank
What Does This Do
This instruments Resilience4j 2.x to make it visible in traces.
It introduces spans for Resilience4j function decorators, as well as tags representing circuit breaker and retry configuration and metrics snapshots.
It's targeting circuit breaker, retry, and fallback decorators in the core and reactor modules.
Motivation
Provide visibility of circuit breaker and retry scopes and attributes in traces.
Additional Notes
Resilience4J spans are not measured by default, but this can be enabled explicitly by
DD_RESILIENCE4J_MEASURED_ENABLED.The circuit breaker and retry snapshot metric tags can be enabled with
DD_RESILIENCE4J_TAG_METRICS_ENABLED.Example
DD_RESILIENCE4J_TAG_METRICS_ENABLED).Contributor Checklist
type:and (comp:orinst:) labels in addition to any usefull labelsclose,fixor any linking keywords when referencing an issue.Use
solvesinstead, and assign the PR milestone to the issueJira ticket: OLDAIDM-540