Skip to content
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

Add excluded_urls to asgi instrumentation docs #2407

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

andy-edvalson
Copy link

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes # (issue)

Type of change

This is purely a documentation update

How Has This Been Tested?

We ran into a need to filter health checks out of our ASGI application's telemetry. I found that the filtering was already supported but not present in the docs.

Does This PR Require a Core Repo Change?

No

Checklist:

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

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

Copy link

linux-foundation-easycla bot commented Apr 10, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@andy-edvalson andy-edvalson changed the title Add extended_urls to asgi instrumentation docs Add excluded_urls to asgi instrumentation docs Apr 11, 2024
Add missing docs for other constructor params
@andy-edvalson andy-edvalson force-pushed the add-excluded-urls-docs branch from 03d2ad5 to e9ad6c6 Compare April 11, 2024 23:12
@xrmx xrmx added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Apr 15, 2024
@xrmx
Copy link
Contributor

xrmx commented Apr 15, 2024

@andy-edvalson please fix tox -e lint and tox -e docs

@andy-edvalson
Copy link
Author

@andy-edvalson please fix tox -e lint and tox -e docs

Sorry just getting back to this. I didn't have access to a machine I could install the extra pieces on to test it

@andy-edvalson andy-edvalson requested a review from a team April 21, 2024 02:30
@@ -449,24 +450,34 @@ class OpenTelemetryMiddleware:

Args:
app: The ASGI application callable to forward requests to.
excluded_urls: Optional parameter to specify URLs that should be excluded from tracing.
This can be useful for skipping health checks or other endpoints that do not need to be monitored.
Should be an instance of :class:`opentelemetry.util.http.ExcludeList`. Defaults to None.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think sphinx forces you to add the automodule for the http utils because of this reference here. I think there are some apis here that we are okay with exposing so adding a docs section for the utils should be no issue.

@lzchen
Copy link
Contributor

lzchen commented Apr 23, 2024

@andy-edvalson

Thanks for the pr! Would you be able to add a section in the configuration docstrings to show the usage? Just like what we have for requests

@andy-edvalson
Copy link
Author

@andy-edvalson

Thanks for the pr! Would you be able to add a section in the configuration docstrings to show the usage? Just like what we have for requests

Yeah I can do that but I won't be able to get back to this for a few days

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.

4 participants