Skip to content

Conversation

@herin049
Copy link
Contributor

@herin049 herin049 commented Dec 5, 2025

Description

Add type checking support for the opentelemetry-aiohttp-client package.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

tox -e typecheck

Does This PR Require a Core Repo Change?

  • Yes. - Link to PR:
  • No.

Checklist:

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

  • Followed the style guidelines of this project
  • Changelogs have been updated

@herin049 herin049 requested a review from a team as a code owner December 5, 2025 05:07
@xrmx xrmx moved this to Ready for review in @xrmx's Python PR digest Dec 5, 2025
_set_http_peer_port_client,
_set_http_url,
_set_status,
_set_http_host_client, # type: ignore[reportUnknownVariableType]
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we avoid adding these ignore if opentelemetry.instrumentation._semconv had more type hints added to it first?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I initially wanted to scope the changes just to aiohttp-client, but I can certainly add typing here too to address the point you're making

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated all the typing inside opentelemetry.instrumentation._semconv :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, and sorry for scope creep. I figured it would help for the other instrumentors later that also use _semconv

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Ready for review

Development

Successfully merging this pull request may close these issues.

2 participants