-
Notifications
You must be signed in to change notification settings - Fork 656
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
Fix autoinstrumentation docs #544
Fix autoinstrumentation docs #544
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.
A little feedback on the guide itself, and curious about eliminating some of the examples since they're a little redundant. with other docs we have.
I'll wait for a decision on #547 before performing further updates to this PR. |
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 added a commit with a couple fixes and one update that takes #567 into consideration, feel free to take it into consideration, @mauriciovasquezbernal 👍
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 as it is now 👍
|
||
As you can see, both outputs are equivalent since the automatic | ||
instrumentation does what the manual instrumentation does too. | ||
|
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 we add more advanced uses cases here? For example passing in configuration arguments in instrument
.
- Convert file to rst to be included in the generated documentation - Include docs for main module. - Use internal reference instead of read the docs link
I wanted to fix this as part of #547, but that will take longer than expected. Given that:
I'd propose to merge this PR that is just changing the file format + fixing some small issues. Further modifications to the content itself could be done in follow up PRs. @toumorokoshi what do you think? Btw, I got so many conflicts that I decided to start from scratch and force pushed to this PR. |
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, thanks!
- Convert file to rst to be included in the generated documentation - Include docs for main module. - Use internal reference instead of read the docs link
Two small fixes for autoinstrumentation docs.