Skip to content

Add aiobotocore instrumentation#1709

Closed
dacevedo12 wants to merge 1 commit intoopen-telemetry:mainfrom
dacevedo12:aiobotocore
Closed

Add aiobotocore instrumentation#1709
dacevedo12 wants to merge 1 commit intoopen-telemetry:mainfrom
dacevedo12:aiobotocore

Conversation

@dacevedo12
Copy link
Copy Markdown

@dacevedo12 dacevedo12 commented Mar 3, 2023

Description

Added instrumentor for aiobotocore, an async client for amazon services.

It's based on the sync botocore instrumentor, overriding the methods to patch the async codepaths

Fixes #553

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?

  • I've been using it on production for a while with no problems.
  • workflow tests should pass to be sure

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
  • Unit tests have been added
  • Documentation has been updated

@dacevedo12 dacevedo12 requested a review from a team March 3, 2023 23:01
@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla Bot commented Mar 3, 2023 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: dacevedo12 / name: David Acevedo (cd6ba57)

@github-actions github-actions Bot requested a review from nemoshlag March 4, 2023 00:37
@dacevedo12
Copy link
Copy Markdown
Author

dacevedo12 commented Mar 4, 2023

CI is all green in my branch now ✔️

nemoshlag
nemoshlag reviewed Mar 5, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@nemoshlag nemoshlag left a comment

Choose a reason for hiding this comment

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

LGTM but I got assigned due to docs/instrumentation ownership (which looks great). @mariojonke can you take a look? I believe you'll this is your domain 🙏

mariojonke
mariojonke reviewed Mar 7, 2023
View reviewed changes
Comment thread CHANGELOG.md
Comment thread instrumentation/opentelemetry-instrumentation-aiobotocore/pyproject.toml Outdated
Comment thread ...emetry-instrumentation-aiobotocore/src/opentelemetry/instrumentation/aiobotocore/__init__.py Outdated
Comment thread ...emetry-instrumentation-aiobotocore/src/opentelemetry/instrumentation/aiobotocore/__init__.py
Comment thread ...emetry-instrumentation-aiobotocore/src/opentelemetry/instrumentation/aiobotocore/__init__.py Outdated
Comment thread instrumentation/opentelemetry-instrumentation-aiobotocore/tests/test_aiobotocore_dynamodb.py
@dacevedo12
Copy link
Copy Markdown
Author

dacevedo12 commented Mar 8, 2023

@nemoshlag it's ok if you want to merge the other one first as this will certainly cause a conflict. I can rebase and update

@nemoshlag
Copy link
Copy Markdown
Member

nemoshlag commented Mar 9, 2023

Hi @dacevedo12 thanks but it's ok. I think new instrumentations should be promoted, but I don't have any control over merging prioritization :)

@dacevedo12
Copy link
Copy Markdown
Author

dacevedo12 commented Mar 15, 2023

Perhaps the bot requested more reviews due to the spellcheck fix in boto3sqs. I just changed it so the CI would pass

Any chance this makes it to the 0.38 release 😄 ?

@dacevedo12 dacevedo12 mentioned this pull request May 2, 2023
Closed
@ronlut
Copy link
Copy Markdown

ronlut commented Aug 20, 2023

@dacevedo12 thanks for your precious time and work :)
I am waiting for this as well, is it still blocked? Who should approve it?
Maybe worth releasing it as a separate instrumentor if it's not gonna happen in this repo?

@dacevedo12
Copy link
Copy Markdown
Author

dacevedo12 commented Aug 20, 2023 •
edited
Loading

They likely won't merge it, check the (ongoing?) discussion at #1714.

I'm down to publish this on a separate repo, but won't happen immediately as I'm busy with some stuff.

Feel free to fork/copypaste in the meantime, that's what I did at work.

@dacevedo12
Copy link
Copy Markdown
Author

dacevedo12 commented Oct 11, 2023 •
edited
Loading

Will move this to my own repo soon. Progress surely won't be made here.

Disappointing.

@dacevedo12 dacevedo12 closed this Oct 11, 2023
@idonava
Copy link
Copy Markdown

idonava commented Feb 25, 2024

Is there any chance to merge this PR?
This library is widely used, and we were also waiting for it.

@henocdz
Copy link
Copy Markdown

henocdz commented May 27, 2024

@lzchen @ocelotl I see you have been working on reviewing and approving recent MRs on this repo; do you know what's required to get this much-needed change? How can we help? It'd be nice to have this available.

Thanks for your work maintaining this.

@herin049 herin049 mentioned this pull request Dec 22, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nikosokolik nikosokolik Awaiting requested review from nikosokolik

@oxeye-nikolay oxeye-nikolay Awaiting requested review from oxeye-nikolay

2 more reviewers

@nemoshlag nemoshlag nemoshlag left review comments

@mariojonke mariojonke mariojonke approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@nemoshlag nemoshlag

@dacevedo12 dacevedo12

@nikosokolik nikosokolik

@oxeye-nikolay oxeye-nikolay

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add support for boto3 instrumentation

8 participants

@dacevedo12 @nemoshlag @ronlut @idonava @henocdz @mariojonke @nikosokolik @oxeye-nikolay