-
Notifications
You must be signed in to change notification settings - Fork 813
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
Update to SDK 0.17.0 #2338
Update to SDK 0.17.0 #2338
Conversation
…nstrumentation into 0.17.0
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.
Awesome 🥳
return extract(GlobalOpenTelemetry.getPropagators(), carrier, getter); | ||
} | ||
|
||
public <C> Context extract(C carrier, TextMapPropagator.Getter<C> getter) { | ||
public <C> Context extract(C carrier, TextMapGetter<C> getter) { |
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.
this is a nice improvement 👍
@@ -35,7 +35,7 @@ void resetOpenTelemetry() { | |||
public void shouldCreateNoopRequestIfNoPropagatorsSet() throws IOException { | |||
// given | |||
InputStream mock = mock(InputStream.class); | |||
GlobalOpenTelemetry.set(OpenTelemetry.getDefault()); | |||
GlobalOpenTelemetry.set(OpenTelemetry.noop()); |
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.
😁
@@ -12,7 +12,7 @@ | |||
public final class BaggageBridging { | |||
|
|||
public static Baggage toApplication(io.opentelemetry.api.baggage.Baggage agentBaggage) { | |||
BaggageBuilder applicationBaggageBuilder = Baggage.builder().setNoParent(); | |||
BaggageBuilder applicationBaggageBuilder = Baggage.builder(); |
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.
this is better too 👍
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.
Yeah I was quite happy with how baggage turned out here and in the SDK's unit tests too, much simpler API.
|
||
// Reread after our calls to setProperty to determine whether we need to enforce agent defaults. | ||
Properties environmentPropertiesWithCopies = | ||
new ConfigBuilder() | ||
.readEnvironmentVariables() | ||
.readSystemProperties() | ||
.build() | ||
.asJavaProperties(); | ||
|
||
// Agent has different defaults than SDK | ||
if (!environmentPropertiesWithCopies.containsKey("otel.propagators")) { | ||
System.setProperty("otel.propagators", "tracecontext,baggage"); | ||
} | ||
String traceExporter = environmentPropertiesWithCopies.getProperty("otel.traces.exporter"); | ||
if (traceExporter == null) { | ||
System.setProperty("otel.traces.exporter", "otlp"); | ||
} else if (traceExporter.equals("none")) { | ||
System.clearProperty("otel.traces.exporter"); | ||
} | ||
String metricExporter = environmentPropertiesWithCopies.getProperty("otel.metrics.exporter"); | ||
if (metricExporter == null) { | ||
System.setProperty("otel.metrics.exporter", "otlp"); | ||
} else if (metricExporter.equals("none")) { | ||
System.clearProperty("otel.metrics.exporter"); | ||
} |
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.
🎉
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 don't understand this :) Why is this not needed anymore?
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.
The SDK autoconfigure now follows the same defaults as the spec, which are the same defaults our agent expected.
No description provided.