-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Go: Roll the dice #3310
Go: Roll the dice #3310
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
@cartermp, please consider this orthogonally
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall this looks great. I think it'd be helpful to have an ending section called Next Steps, like the python docs here: https://deploy-preview-3310--opentelemetry.netlify.app/docs/instrumentation/python/getting-started/#next-steps
Doesn't have to be copied verbatim -- I think mainly we should link to the manual instrumentation page and the exporters page.
You will learn how you can instrument a simple application automatically, in | ||
such a way that [traces][] and [metrics][] are emitted to the console. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You will learn how you can instrument a simple application automatically, in | |
such a way that [traces][] and [metrics][] are emitted to the console. | |
You will learn how you can instrument a simple application using | |
instrumentation libraries and custom instrumentation with | |
OpenTelemetry APIs. You'll create [traces][] and [metrics][] and emit | |
them to the console. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bigger suggestion here -- since we're not using the go autoinstrumentation agent (maybe that comes in the future!) I think it's worth rephrasing here to emphasize libraries and custom instrumentation with APIs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I also mention the SDK? Isn't the term "OpenTelemetry APIs confusing, because there is also OpenTelemetry API which is used for custom instrumentation?
I will simply change to
You will learn how you can instrument a simple application automatically, in | |
such a way that [traces][] and [metrics][] are emitted to the console. | |
You will learn how you can instrument a simple application manually, in | |
such a way that [traces][] and [metrics][] are emitted to the console. |
But I am open for further suggestions.
PS. I have created #3308 before this PR and I have forgotten to remove "automatically" 🤦
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fine. I will add it. See 4c82660 🤣 |
Co-authored-by: Phillip Carter <pcarter@fastmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm overall
@cartermp I think it is good to merge. |
Towards #2623
Towards open-telemetry/opentelemetry-go#4531
Rewrite of: https://opentelemetry.io/docs/instrumentation/go/getting-started/
Preview: https://deploy-preview-3310--opentelemetry.netlify.app/docs/instrumentation/go/getting-started/
Based on open-telemetry/opentelemetry-go#4539