-
Notifications
You must be signed in to change notification settings - Fork 16
Bootstrap docs for otel mappings troubleshooting #140
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
base: staging
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for suse-obs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| start_page: en:classic.adoc | ||
| asciidoc: | ||
| attributes: | ||
| stackpacks2_enabled: false |
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.
is there now a deploy where this is enabled? should we get that going?
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.
No, there's no deployment with that enabled. @akashraj4261 explained to us that we could flip it for the review so we can see it in the 😎 Deploy Preview
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 would be great if DeployPreview would enable the flag but our production deploy does not, lets ask him during standup
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 see netlify uses the ss-local-playbook to render the netlify deploy. (https://app.netlify.com/projects/suse-obs/deploys/69285d3149b414000837bf17)
Can we not add the attribute to true to that file and leave it out of the ss-remote-playbook (which i assume is used for production).
@akashraj4261 please advise
| ifdef::stackpacks2_enabled[] | ||
| ** xref:setup/otel/otelmappings/README.adoc[Open Telemetry Mappings] | ||
| *** xref:setup/otel/otelmappings/concepts.adoc[Concepts] | ||
| *** xref:setup/otel/otelmappings/troubleshooting.adoc[Troubleshooting] |
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 can still find this page through search, any chance we can feature flag all the pages? https://deploy-preview-140--suse-obs.netlify.app/suse-observability/latest/en/setup/otel/otelmappings/troubleshooting.html
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 can feature flag the whole page with a similar ifdef block but we discussed that's it's an overkill. Same if you would like to guard the case where somebody knows/gets the url
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.
@rb3ckers what is your thought here. are we ok having these pages show up in live docs during search and if the URLs are known?
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.
Seems that feature flagging each page will help with the search ..
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 prefer not to have them in the search. But having the url be accessible is ok I think. If people are guessing URLs I wouldn't mind too much.
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.
Then I think the latest changes accomplish that .... not available in the search
No description provided.