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

Extending with custom manual instrumentation #5789

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

Conversation

zeitlinger
Copy link
Member

Fixes #5713

@zeitlinger zeitlinger self-assigned this Dec 13, 2024
@zeitlinger zeitlinger requested a review from a team as a code owner December 13, 2024 14:27
@opentelemetrybot opentelemetrybot requested review from a team December 13, 2024 14:27
Copy link
Member

@svrnm svrnm left a comment

Choose a reason for hiding this comment

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

thanks

content/en/docs/zero-code/java/agent/annotations.md Outdated Show resolved Hide resolved
content/en/docs/zero-code/java/agent/api.md Outdated Show resolved Hide resolved
content/en/docs/zero-code/java/spring-boot-starter/api.md Outdated Show resolved Hide resolved
@zeitlinger
Copy link
Member Author

@svrnm can you check?

Copy link
Member

@svrnm svrnm left a comment

Choose a reason for hiding this comment

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

One more thing, I am not sure if api is the right name for that, since we do not even mention "api" in the title, description, so either we do that, e.g. "Adding custom instrumentations with the API" or we rename the file to something like "extending-instrumentation.md" as well. wdyt?

@zeitlinger
Copy link
Member Author

One more thing, I am not sure if api is the right name for that, since we do not even mention "api" in the title, description, so either we do that, e.g. "Adding custom instrumentations with the API" or we rename the file to something like "extending-instrumentation.md" as well. wdyt?

it's API to separate it from annotations - which also extend

@zeitlinger
Copy link
Member Author

@svrnm please check again

@zeitlinger
Copy link
Member Author

@svrnm can you check?

@svrnm
Copy link
Member

svrnm commented Dec 20, 2024

@svrnm can you check?

I did, but I'd like to get some feedback from @open-telemetry/java-approvers or @open-telemetry/java-instrumentation-approvers as well

@zeitlinger
Copy link
Member Author

@jack-berg can you check?

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.

I think the sections for the Java agent and Spring Boot starter are great additions.

I only have question about how to structure the addition to the general API page.

content/en/docs/zero-code/java/spring-boot-starter/api.md Outdated Show resolved Hide resolved
content/en/docs/zero-code/java/spring-boot-starter/api.md Outdated Show resolved Hide resolved
content/en/docs/zero-code/java/agent/annotations.md Outdated Show resolved Hide resolved
content/en/docs/zero-code/java/agent/api.md Outdated Show resolved Hide resolved
content/en/docs/zero-code/java/agent/api.md Outdated Show resolved Hide resolved
Comment on lines 602 to +603

### GlobalOpenTelemetry
#### GlobalOpenTelemetry
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure about the organization of the additions in this file

Maybe could just add two small notes right above here, something like "If you are using the Java agent, see X for information about how to obtain an OpenTelemetry instance" (and similar note for Spring Boot starter)

Copy link
Member

Choose a reason for hiding this comment

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

I agree - I think an alert could be good. Something like

{{% alert title="Spring Boot Starter" %}} 
The Spring Boot starter is a special case where [...].
{{% /alert %}}

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it's useful if to have a hint it the ToC - which is not possible with an info box

image

Copy link
Member Author

Choose a reason for hiding this comment

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

@svrnm do you have a recommendation?

@opentelemetrybot opentelemetrybot requested review from a team January 6, 2025 10:55
@zeitlinger zeitlinger force-pushed the extending-with-custom-manual-instrumenation branch from 1e96fae to 217dc46 Compare January 6, 2025 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

how to get a meter/tracer in Java agent or spring stater
5 participants