-
Notifications
You must be signed in to change notification settings - Fork 564
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
3.x Backport of scope/baggage fix #8244
3.x Backport of scope/baggage fix #8244
Conversation
Signed-off-by: Tim Quinn <[email protected]>
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.
Couple of minor comments
@@ -27,6 +27,10 @@ | |||
<artifactId>helidon-tracing-opentelemetry</artifactId> | |||
<name>Helidon Tracing Open Telemetry</name> | |||
|
|||
<properties> | |||
<version.lib.opentelemetry-sdk-extension-autoconfigure>1.22.0-alpha</version.lib.opentelemetry-sdk-extension-autoconfigure> |
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 assume we're OK with this property at this level, and not in dependencies/pom.xml
as would normally be the case.)
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.
My understanding is that for "tangential" dependencies we don't want to load up dependencies/pom.xml
with them.
Now, maybe OTel is becoming central enough that we do want them there but ideally there should be direct dependencies on OTel only from one (maybe two?) artifacts.
Maybe that's just for integrations and this usage is less tangential than those.
@tomas-langer @romain-grecourt Thoughts, folks?
...pentelemetry/src/main/java/io/helidon/tracing/opentelemetry/MutableOpenTelemetryBaggage.java
Outdated
Show resolved
Hide resolved
…lude baggage in span's context
} | ||
|
||
void baggage(String key, String value) { | ||
values.put(key, new HBaggageEntry(value, new HBaggageEntryMetadata(""))); |
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.
Not thread safe. OK?
Description
Resolves #8211
Backport of PR #8225 - see the description there for full information.
Documentation
Bug fix - no doc impact.