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.

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

Move the Spring Auto-Configuration doc to opentelemetry.io #3758

Closed

Conversation

jaydeluca
Copy link
Member

@jaydeluca jaydeluca commented Jan 11, 2024

Copy link
Member

@jeanbisutti jeanbisutti left a comment

Choose a reason for hiding this comment

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

@jaydeluca Thank you for your patience.

The main usage is the OpenTelemetry starter. It is also possible to use the Zipkin and the Jaeger exporters, or Spring autoconfiguration (not starters). I would suggest to focus documentation on the OpenTelemetry starter and after mention other possible configurations with Spring.

We could structure the documentation in this way:

## OpenTelemetry Spring starter


## Other configurations

Instead of using the OpenTelemetry Spring starter, you can use the OpenTelemetry autoconfiguration features with an annotation, or the Jaeger and Zipkin exporters.

### Spring autoconfiguration

### OpenTelemetry Jaeger Exporter Starter

### OpenTelemetry Zipkin Exporter Starter

I have added comments on the PR to move the different parts to this structure.

WDYT?

The OpenTelemetry Jaeger Exporter Starter would report the https://github.com/open-telemetry/opentelemetry-java-instrumentation/tree/main/instrumentation/spring/starters/jaeger-spring-boot-starter page.

The OpenTelemetry Zipkin Exporter Starter would report the https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/instrumentation/spring/starters/zipkin-spring-boot-starter/README.md page.

@jeanbisutti
Copy link
Member

@jaydeluca The Jaeger exporter and Jaeger Spring Boot starter will be removed from the 2.0 version (https://github.com/open-telemetry/opentelemetry-java-instrumentation/pull/10241/files). I think we could remove all the Jaeger content from this PR.

@jaydeluca
Copy link
Member Author

@jeanbisutti thank you for all the guidance and feedback. I believe I have made the changes according to your recommendations but please let me know if I've misunderstood if there are any additional changes or areas to work on further

Copy link
Member

@jeanbisutti jeanbisutti left a comment

Choose a reason for hiding this comment

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

@jaydeluca I have added comments and suggestions. Things have changed since 2.0 release.

Something bad may have happened with the merge?

image

content/en/docs/languages/java/automatic/spring-boot.md Outdated Show resolved Hide resolved
content/en/docs/languages/java/automatic/spring-boot.md Outdated Show resolved Hide resolved
content/en/docs/languages/java/automatic/spring-boot.md Outdated Show resolved Hide resolved
content/en/docs/languages/java/automatic/spring-boot.md Outdated Show resolved Hide resolved
content/en/docs/languages/java/automatic/spring-boot.md Outdated Show resolved Hide resolved
content/en/docs/languages/java/automatic/spring-boot.md Outdated Show resolved Hide resolved
content/en/docs/languages/java/automatic/spring-boot.md Outdated Show resolved Hide resolved
content/en/docs/languages/java/automatic/spring-boot.md Outdated Show resolved Hide resolved
content/en/docs/languages/java/automatic/spring-boot.md Outdated Show resolved Hide resolved
@jaydeluca jaydeluca force-pushed the update-spring-boot-docs branch from f732304 to 7358798 Compare January 18, 2024 14:40
@jeanbisutti
Copy link
Member

/fix:format

Copy link
Contributor

@jaydeluca jaydeluca marked this pull request as ready for review January 19, 2024 12:00
@jaydeluca jaydeluca requested review from a team January 19, 2024 12:00
@chalin chalin force-pushed the update-spring-boot-docs branch from e0f90e1 to cf6c4f6 Compare January 19, 2024 12:10
Copy link
Contributor

@chalin chalin left a comment

Choose a reason for hiding this comment

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

An initial copyedit pass 👍🏻

content/en/docs/languages/java/automatic/spring-boot.md Outdated Show resolved Hide resolved
content/en/docs/languages/java/automatic/spring-boot.md Outdated Show resolved Hide resolved
content/en/docs/languages/java/automatic/spring-boot.md Outdated Show resolved Hide resolved
content/en/docs/languages/java/automatic/spring-boot.md Outdated Show resolved Hide resolved
content/en/docs/languages/java/automatic/spring-boot.md Outdated Show resolved Hide resolved
content/en/docs/languages/java/automatic/spring-boot.md Outdated Show resolved Hide resolved
content/en/docs/languages/java/automatic/spring-boot.md Outdated Show resolved Hide resolved
content/en/docs/languages/java/automatic/spring-boot.md Outdated Show resolved Hide resolved
content/en/docs/languages/java/automatic/spring-boot.md Outdated Show resolved Hide resolved
content/en/docs/languages/java/automatic/spring-boot.md Outdated Show resolved Hide resolved
Copy link
Member

@jeanbisutti jeanbisutti left a comment

Choose a reason for hiding this comment

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

Thanks @jaydeluca!

I would let the ### Additional instrumentations part in the OpenTelemetry starter. The reason is most users should be interested by the OpenTelemetry starter. The Other Configurations part is for things that should not interest mot users.

So, we would have something like this:

OpenTelemetry Spring starter

Automatic instrumentation with Spring auto configurations

Additional instrumentations

JDBC Instrumentation

Logging Instrumentation

Instrumentation Annotations

Other instrumentations

@jaydeluca
Copy link
Member Author

/fix:format

Copy link
Contributor

jaydeluca and others added 25 commits January 30, 2024 20:05
…pen-telemetry#3869)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Phillip Carter <[email protected]>
…etry#3870)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…pen-telemetry#3871)

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Phillip Carter <[email protected]>
@jaydeluca
Copy link
Member Author

wow I apologize everyone, I must have taken a wrong turn when rebasing this and don't know how I ended up in this state with everyone added as a participant. I'm really sorry.

@jaydeluca
Copy link
Member Author

closing this PR and continuing via: #3900 to avoid spamming everyone with notifications. Sorry again.

@jaydeluca jaydeluca closed this Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.