-
Notifications
You must be signed in to change notification settings - Fork 1.5k
docs: add highlight to js tracer examples #7937
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
Conversation
This is a pretty big edit. @open-telemetry/javascript-maintainers Could you have a look? |
the idea was to do a step by step, setting up things, etc, I'm not sure if the removal will cause confusion |
I see where you are coming from, but let me point out that at the top of that section we have a short version: https://opentelemetry.io/docs/languages/js/instrumentation/#acquiring-a-tracer
The part below you are referring to is for the "walk along tutorial", and people might just want to copy and paste that over. A way to resolve that is that we use code highlights provided by hugo, so the lines that have been changed get easier to be seen: https://gohugo.io/content-management/syntax-highlighting/#fenced-code-blocks see the "hl_lines" option |
I think even with the help of highlighting, the problem of high noise-to-signal is still not resolved. The "rolldice" logic below the tracer acquirement complex and does more distraction than clarifying. Maybe the culprit here is that the "rolldice" example is too complex? Also do we really need two files and two tracers to explain how to use a tracer? Can we keep it simple and focused, by using an one file example? |
Having the 2 files is part of the the point we want to drive with the example, you have an "app" file that leverages the API and the SDK, and you have a "library" file that only depends on the "API", which means that if you for example develop a 3rd party library it should only take dependency on the API. I understand that this is far from optimal, we can definitely reduce some of the overhead with highlighting lines or maybe even not showing the full files, or we can make that part of the docs visually distinct, but removing this portion is not an option. |
Sounds good. I changed the PR to adding highlights. |
@open-telemetry/javascript-approvers PTAL! |
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.
thanks!
Thank you for your contribution @chiawendt! 🎉 We would like to hear from you about your experience contributing to OpenTelemetry by taking a few minutes to fill out this survey. |
The removed code example is confusing because the acquired tracer is unused. Merging acquisition and usage makes examples more readable.