Skip to content
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.

Sign up for GitHub

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

Jump to bottom

demonstrate ExtendedTracer #288

Merged
merged 4 commits into from
Feb 2, 2024
Merged

demonstrate ExtendedTracer #288

merged 4 commits into from
Feb 2, 2024

Conversation

zeitlinger
Copy link
Member

which was newly introduced in 1.33

@zeitlinger zeitlinger requested review from a team December 19, 2023 11:18
@zeitlinger zeitlinger self-assigned this Dec 19, 2023
breedx-splk
breedx-splk reviewed Jan 4, 2024
View reviewed changes
manual-tracing/src/main/java/io/opentelemetry/example/tracing/ExampleConfiguration.java
OpenTelemetrySdk sdk =
OpenTelemetrySdk.builder()
.setTracerProvider(sdkTracerProvider)
.setPropagators(ContextPropagators.create(W3CTraceContextPropagator.getInstance()))
Copy link
Contributor

Choose a reason for hiding this comment

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

This is configured by default, isn't it?

Copy link
Member Author

Choose a reason for hiding this comment

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

in auto-configure it is, but not in the builder, which is used here.

breedx-splk
breedx-splk approved these changes Jan 4, 2024
View reviewed changes
Copy link
Contributor

@breedx-splk breedx-splk left a comment

Choose a reason for hiding this comment

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

This is looking good to me. I think the http examples demonstrate the value here nicely, but I could possibly see it being contentious to use the existing http example with the new incubating tracer.

In any case, nice work.

@zeitlinger
Copy link
Member Author

This is looking good to me. I think the http examples demonstrate the value here nicely, but I could possibly see it being contentious to use the existing http example with the new incubating tracer.

I see the point.
I went with this approach to avoid duplicating the example.

@zeitlinger
Copy link
Member Author

@breedx-splk can you check again?

breedx-splk
breedx-splk approved these changes Jan 18, 2024
View reviewed changes
Copy link
Contributor

@breedx-splk breedx-splk left a comment

Choose a reason for hiding this comment

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

Yup, still looking good. Thanks.

@zeitlinger
Copy link
Member Author

@jack-berg can you merge?

jack-berg
jack-berg reviewed Jan 18, 2024
View reviewed changes
http/src/main/java/io/opentelemetry/example/http/HttpServer.java
private static final Tracer tracer =
openTelemetry.getTracer("io.opentelemetry.example.http.HttpServer");
private static final ExtendedTracer tracer =
ExtendedTracer.create(openTelemetry.getTracer("io.opentelemetry.example.http.HttpServer"));
Copy link
Member

Choose a reason for hiding this comment

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

Its arguably a bit strange to reference experimental APIs as part of canonical examples, but I think its fine if we're serious about incubating these to eventually incorporate into Tracer.

@zeitlinger
Copy link
Member Author

@jack-berg can you check again?

@zeitlinger
Copy link
Member Author

@trask can you merge?

@trask trask merged commit 663dc84 into open-telemetry:main Feb 2, 2024
@zeitlinger zeitlinger deleted the extended-tracer branch April 9, 2024 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jack-berg jack-berg jack-berg approved these changes

@breedx-splk breedx-splk breedx-splk approved these changes

Assignees

@zeitlinger zeitlinger

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@zeitlinger @jack-berg @breedx-splk @trask