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

Rework the java exporters page #3558

Merged
merged 5 commits into from
Nov 27, 2023
Merged

Rework the java exporters page #3558

merged 5 commits into from
Nov 27, 2023

Conversation

svrnm
Copy link
Member

@svrnm svrnm commented Nov 17, 2023

Based on #3355 I updated the java exporters documentation to have a similar structure.

@svrnm svrnm requested review from a team November 17, 2023 11:39
@svrnm svrnm added the sig:java label Nov 17, 2023
@svrnm svrnm mentioned this pull request Nov 17, 2023
11 tasks
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.

I gave it a read-through. Looks good from here. 👍🏻

Copy link
Member

@jack-berg jack-berg left a comment

Choose a reason for hiding this comment

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

Just left a few comments but looks pretty good. 👍

Copy link
Contributor

@cartermp cartermp left a comment

Choose a reason for hiding this comment

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

LGTM pending feedback addressed from @jack-berg

@svrnm
Copy link
Member Author

svrnm commented Nov 27, 2023

@jack-berg / @open-telemetry/java-approvers PTAL!

Copy link
Member

@jack-berg jack-berg left a comment

Choose a reason for hiding this comment

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

Looks good with the recommendations from @svrnm.

Copy link
Member

@trask trask left a comment

Choose a reason for hiding this comment

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

we generally recommend using the autoconfigure API, do you think that could work on this page? (i'm also totally good with this PR as-is and we can always explore converting it to autoconfigure API later)

content/en/docs/instrumentation/java/exporters.md Outdated Show resolved Hide resolved
@svrnm
Copy link
Member Author

svrnm commented Nov 27, 2023

we generally recommend using the autoconfigure API, do you think that could work on this page? (i'm also totally good with this PR as-is and we can always explore converting it to autoconfigure API later)

I would say so, yes. For Node.JS we finally also agreed that we do not provide that many instructions for the "low level" API but using NodeSDK with all the automatic wiring. @open-telemetry/docs-approvers WDYT?

Signed-off-by: svrnm <[email protected]>
@svrnm svrnm merged commit 3924d97 into main Nov 27, 2023
16 checks passed
@svrnm svrnm deleted the java-exporters branch November 27, 2023 21:21
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.

5 participants