-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[IA] Rename /docs/instrumentation to /docs/languages, and Manual subpages to Instrumentation #3761
[IA] Rename /docs/instrumentation to /docs/languages, and Manual subpages to Instrumentation #3761
Conversation
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.
Sounds reasonable, as it leaves the door open to further variants, such as "automatic instrumentation". I do wonder, though: Isn't this sort of assuming that the code based instrumentation is the default way of instrumenting? That might not be true for runtime languages like Node or Java.
I like @pellared 's structure here, which might inform a deeper restructuring of our docs: #3228 (comment)
I'll make a strong statement now and say, yes, code based instrumentation is the default way, because telemetry should be built-in, so agents and instrumentation libraries are only an intermediate thing until otel is native everywhere. We see this actually happening for Java with the Spring Boot Starter and for Node.Js with nextjs having otel built-in natively (and by that not needing any instrumentation library anymore)
I was thinking about this as well, because "Instrumentation" right now also includes "Bootstrap/Setup the SDK" right now, which probably should be extracted (maybe in it's own PR). We have something for "How you "instrument libraries" which is called "Libraries" in some of the languages. |
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.
I like this! 👍🏻
netlify.toml
Outdated
[[redirects]] | ||
from = "/docs/instrumentation/*" | ||
to = "/docs/languages/:splat" | ||
force = true |
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.
As discussed in Slack, add this rule to the new languages index file instead. Follow the Registry page as an example.
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.
Add the following to the Languages index front matter:
aliases: [/docs/instrumentation]
redirects: [{ from: /docs/instrumentation/*, to: ':splat' }]
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!
I like the concept of the change. I'm happy to proceed with this moving out of a draft for review. |
I think it is a great improvement. |
The main thing in my mind here is:
It looks a little odd without some tweaks to both the automatic sections and the (if relevant) libraries sections. That's a separate issue to tackle, but it needs tackling. |
Agreed, we need to review the other section titles as well! Let's have some follow up PRs then |
Signed-off-by: svrnm <neumanns@cisco.com>
Signed-off-by: svrnm <neumanns@cisco.com>
Signed-off-by: svrnm <neumanns@cisco.com>
f83ddd9
to
2f38f7a
Compare
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.
Oops, the redirects for manual
pages aren't implemented correctly. Let me see how we can fix this.
@svrnm - actually, if you'd like to merge this now, I can fix the redirects in a followup PR. WDYT? |
Signed-off-by: svrnm <neumanns@cisco.com>
as discussed via slack I try to fix them right now because there are a few other things that need to be updated (links on the status page) |
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. In particular the redirects are fixed.
See inline for minor suggestions.
Co-authored-by: Patrice Chalin <chalin@users.noreply.github.com>
/docs/instrumentation
to /docs/languages
, and Manual subpages to Instrumentation
/docs/instrumentation
to /docs/languages
, and Manual subpages to Instrumentation
I was once again trying to wrap my head around #3228, and I wanted to start with a first part of what I have proposed in this comment:
This PR purposefully is not yet touching the word "Automatic", this is worth a separate consideration
Final Note, I kick this off as a draft PR for discussion, because right now this PR only changes
linkTitle
s and if we want to go with this change, we need to rename files & URLs and have to manage a ton of aliases.@open-telemetry/docs-approvers PTAL
Preview: https://deploy-preview-3761--opentelemetry.netlify.app/docs/languages/
Redirect tests (selected):