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 java sdk configuration documentation into docs site #4377

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

jaydeluca
Copy link
Contributor

@jaydeluca jaydeluca commented Apr 27, 2024

Contributes to:

A few notes:

  • Instead of listing both the system properties and environment variables, I just added the env vars. If the system properties would be preferred or if we think there's value in both, let me know and I can update. Other languages seemed to reference env vars so thats why I chose that one, but a lot of the rest of the existing doc used the system properties.
  • I left links out to Batch span processor, Span limits, and Using the SPI to further configure the SDK since I didn't see similar sections in other languages, but let me know if we want all of them ported over and I can do those as well
  • I defaulted to including every config option for each category that I moved over, but if there are things that make sense to just leave documented in the SDK repo vs here, let me know and I can drop them

Preview: https://deploy-preview-4377--opentelemetry.netlify.app/docs/languages/java/automatic/configuration/#resources

@jaydeluca jaydeluca requested review from a team as code owners April 27, 2024 17:48
Copy link
Member

@theletterf theletterf left a comment

Choose a reason for hiding this comment

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

Great work! First pass review follows.

content/en/docs/languages/java/automatic/configuration.md Outdated Show resolved Hide resolved
content/en/docs/languages/java/automatic/configuration.md Outdated Show resolved Hide resolved
content/en/docs/languages/java/automatic/configuration.md Outdated Show resolved Hide resolved
content/en/docs/languages/java/automatic/configuration.md Outdated Show resolved Hide resolved
content/en/docs/languages/java/automatic/configuration.md Outdated Show resolved Hide resolved
content/en/docs/languages/java/automatic/configuration.md Outdated Show resolved Hide resolved
content/en/docs/languages/java/automatic/configuration.md Outdated Show resolved Hide resolved
content/en/docs/languages/java/automatic/configuration.md Outdated Show resolved Hide resolved
content/en/docs/languages/java/automatic/configuration.md Outdated Show resolved Hide resolved
content/en/docs/languages/java/automatic/configuration.md Outdated Show resolved Hide resolved
@theletterf
Copy link
Member

@open-telemetry/java-instrumentation-maintainers PTAL

@svrnm svrnm added the sig-approval-missing Co-owning SIG didn't provide an approval label Apr 30, 2024
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.

One minor drive-by comment regarding use of a "real" (bidirectionally linked) footnote.

@jaydeluca
Copy link
Contributor Author

One minor drive-by comment regarding use of a "real" (bidirectionally linked) footnote.

@chalin I looked around for a pattern to follow for footnotes to ensure consistency and landed on mirroring the Next Steps sections from other pages. If this is not what you were suggesting, please let me know. If this is what you meant, then I can followup with a similar PR to the SDK repo to add a link to the doc site as well.

@svrnm
Copy link
Member

svrnm commented May 13, 2024

@open-telemetry/java-approvers @open-telemetry/java-instrumentation-approvers PTAL!

@chalin
Copy link
Contributor

chalin commented May 13, 2024

Hi @jaydeluca - I've edited the opening comment to indicate that this PR contributes to #4032. We should only close #4032 once the corresponding pieces are removed from that file (I don't know if the entire file is covered here and will therefore be removed, or have a link added to this page). WDYT?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig:java sig-approval-missing Co-owning SIG didn't provide an approval
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants