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

@xrmx
Copy link
Contributor

xrmx commented Dec 10, 2025

@herin049 If you are using aiohttp-client at work, wdyt of adding yourself as component owner?

@herin049
Copy link
Contributor Author

herin049 commented Dec 10, 2025

@herin049 If you are using aiohttp-client at work, wdyt of adding yourself as component owner?

Absolutely. We use aiohttp pretty extensively. We also use AWS Lambda + requests so if component owners are needed for the aws-lambda or requests instrumentation packages, I can be added there too. Let me know what you think, I can create a new PR to add myself to the relevant packages.

Copy link
Contributor

@tammy-baylis-swi tammy-baylis-swi left a comment

Choose a reason for hiding this comment

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

Thank you this is quite helpful!

@herin049 herin049 force-pushed the aiohttp-client-typechecking branch from ee3d706 to 2970295 Compare December 10, 2025 19:03
@herin049 herin049 force-pushed the aiohttp-client-typechecking branch from 2970295 to 417f1a2 Compare December 10, 2025 19:07
@herin049
Copy link
Contributor Author

@herin049 If you are using aiohttp-client at work, wdyt of adding yourself as component owner?

Update the PR to include myself as a component owner for aiohttp-client, I'll add myself as a component owner to other repos in a another PR

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.

3 participants