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.

Sign up for GitHub

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

Jump to bottom

[UNRECOVERABLE] Move instrumentation aiohttp client #48

Closed
wants to merge 10 commits into from
Closed

[UNRECOVERABLE] Move instrumentation aiohttp client #48

wants to merge 10 commits into from

Conversation

NathanielRN
Copy link
Contributor

@NathanielRN NathanielRN commented Oct 19, 2020 •
edited
Loading

Please review #49 instead!

lzchen and others added 9 commits August 3, 2020 10:10
@lzchen
Rename web framework packages from "ext" to "instrumentation" (open-t…
1c970e6 
…elemetry#961)
@lzchen
chore: 0.13.dev0 version update (open-telemetry#991)
a43d088
Span name updated to follow semantic conventions to reduce cardinality (
c9a0fe7 
open-telemetry#972)
@gthb
docs: fix outdated alpha statement (open-telemetry#1047)
ff33a26 
`README.md` and the opentelemetry website say this library is in beta,
and releases have been called betas since March, so update
`docs/index.rst` to be consistent with that.
release: updating changelogs and version to 0.13b0 (open-telemetry#1129)
44dc531 
* updating changelogs and version to 0.13b0
chore: bump dev version (open-telemetry#1131)
ee1805d
@lzchen
Use is_recording flag in asgi, pyramid, aiohttp instrumentation (open…
2b37136 
…-telemetry#1142)
@mariojonke
Add instrumentor and auto instrumentation support for aiohttp (open-t…
a7144ba 
…elemetry#1075)
@lzchen
chore: bump dev version (open-telemetry#1235)
83cbce4
@NathanielRN NathanielRN requested a review from a team October 19, 2020 21:18
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Oct 19, 2020 •
edited
Loading

CLA Check
The committers are authorized under a signed CLA.

@NathanielRN NathanielRN force-pushed the move-instrumentation-aiohttp-client branch from 59ed570 to 8cc2705 Compare October 20, 2020 00:26
bhautikpip
bhautikpip approved these changes Oct 20, 2020
View reviewed changes
Copy link

@bhautikpip bhautikpip left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Files look good! Why this PR has not enabled tests? I don't see tests for this PR in the actions section.

@NathanielRN
Copy link
Contributor Author

@bhautikpip

Thank you!! The only reason I didn't add tests yet (and I didn't plan to until we get all the packages we want in) is because the tests introduced here depend on other packages that will be coming (very soon hopefully!) in future PRs.

For example the files in this PR have these import statements:

from opentelemetry.instrumentation.instrumentor import BaseInstrumentor
from opentelemetry.instrumentation.utils import (
    http_status_to_canonical_code,
    unwrap,
)

Both of which don't exist yet in this repo until I add them, in future PRs.

After the PRs with the packages are merged, I'll create a final PR to add tests. (And I count on it working based on my big #47 PR)

@NathanielRN NathanielRN deleted the move-instrumentation-aiohttp-client branch October 20, 2020 01:56
@NathanielRN
Copy link
Contributor Author

Closed this PR by accident... the exact same PR is in #49

@NathanielRN NathanielRN changed the title Move instrumentation aiohttp client [UNRECOVERABLE] Move instrumentation aiohttp client Oct 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bhautikpip bhautikpip bhautikpip approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@NathanielRN @bhautikpip @lzchen @gthb @mariojonke