Skip to content

Add configuration to requests instrumentation readme #556

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

Merged
merged 3 commits into from
Jul 7, 2021

Conversation

lmolkova
Copy link
Contributor

Description

Added steps to enable requests instrumentation (currently one has to look to integration tests to see how it has to be done)

Type of change

  • This change requires a documentation update

How Has This Been Tested?

created sample app and saw requests instrumentation tracking span

Does This PR Require a Core Repo Change?

  • No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

@lmolkova lmolkova requested review from a team, codeboten and srikanthccv and removed request for a team June 29, 2021 18:22
Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

Normally the docs for this would be available here: https://opentelemetry-python-contrib.readthedocs.io/en/latest/instrumentation/requests/requests.html and look something like https://opentelemetry-python-contrib.readthedocs.io/en/latest/instrumentation/redis/redis.html. This is not working currently because of #552.

@lmolkova
Copy link
Contributor Author

@codeboten thanks for the review and the context! Do I have to update changelog to make pipeline happy?

@srikanthccv srikanthccv added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Jun 29, 2021
@codeboten
Copy link
Contributor

@codeboten thanks for the review and the context! Do I have to update changelog to make pipeline happy?

Nope, looks like @lonewolf3739 already added the skip changelog label, should be good to go.

@codeboten codeboten requested a review from srikanthccv June 30, 2021 15:21
@lzchen
Copy link
Contributor

lzchen commented Jun 30, 2021

This isn't really consistent with the documentation for the rest of our instrumentations (the Configuration sections in django and flask pertains to actual configuration rather than usage). How to use the instrumentations are shown in the docstrings instead.

@lmolkova
Copy link
Contributor Author

This isn't really consistent with the documentation for the rest of our instrumentations (the Configuration sections in django and flask pertains to actual configuration rather than usage). How to use the instrumentations are shown in the docstrings instead.

Ah, I see. the docstrings are not published because of the issue mentioned above.
Should we abandon the idea of having a basic setup in readme.md then? There are other instrumentations (aiohttp client) which has an example in the readme.

@codeboten codeboten merged commit bf97e17 into open-telemetry:main Jul 7, 2021
andresbeckruiz pushed a commit to open-o11y/opentelemetry-python-contrib that referenced this pull request Jul 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants