Skip to content

opentelemetry bridge #1411

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
Merged
merged 46 commits into from
Mar 8, 2022
Merged

opentelemetry bridge #1411

merged 46 commits into from
Mar 8, 2022
+1,332 −158

Conversation

basepi
Copy link
Contributor

@basepi basepi commented Nov 19, 2021

Related issues

Closes #1362
@basepi basepi self-assigned this Nov 19, 2021
@ghost
Copy link

ghost commented Nov 19, 2021 •
edited by ghost
Loading

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-03-08T17:02:53.964+0000

  • Duration: 20 min 59 sec

Test stats 🧪

Test Results
Failed 0
Passed 4824
Skipped 3207
Total 8031

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /test linters : Run the Python linters only.

  • /test full : Run the full matrix of tests.

  • /test benchmark : Run the APM Agent Python benchmarks tests.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)
basepi
basepi commented Dec 1, 2021
View reviewed changes
basepi added 2 commits January 13, 2022 11:32
@basepi
Switch to tuples for execution_context
781dc51 
Didn't actually do the testing I mentioned, but tuples are safer than mutating in place.
@basepi
Switch span ending to unset_span instead of setting parent
e44e195
beniwohli
beniwohli reviewed Jan 14, 2022
View reviewed changes
Copy link
Contributor

@beniwohli beniwohli 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.

Looks great! It would be interesting to know if this has any negative effects on performance. And if it does, I wonder if using a list of tuples instead of a tuple of tuples would help, as we wouldn't be constantly recreating the outer container.
houndci-bot
houndci-bot reviewed Feb 3, 2022
View reviewed changes
basepi added 16 commits February 15, 2022 11:09
@basepi
Add first test (and many fixes for it)
cbcd719
@basepi
Remove compat
b8b1f32
@basepi
Fix other failing tests
4bb37d3
@basepi
Add another test (plus more fixes)
513278e
@basepi
Merge remote-tracking branch 'upstream/main' into otel
27d3fcf
@basepi
Remove contextvars/threadlocal test for py3.6
2678177 
Because I added opentelemetry to reqs-base.txt and it requires the contextvars backport, this test now fails on py3.6, and isn't necessary for newer versions. I think that's acceptable, considering py3.6 is EOL anyway.
@basepi
Don't make Tracer a singleton (should fix tests?)
cd15f03
@basepi
Add test for spans without auto-attach
6031d8b
@basepi
Rename otel_tests.py to tests.py
f2d2754
@basepi
Add more tests
9e9ea9c
@basepi
Docs + a couple of changes to more closely match opentracing bridge
2e0c2ea
@basepi
Merge remote-tracking branch 'upstream/main' into otel
5fc8c0d
@basepi
Deprecate opentracing bridge
a6bbc37
@basepi
More docs
bff1d31
@basepi
Move opentelemetry to its own matrix
b29b2ce
@basepi
Merge remote-tracking branch 'upstream/main' into otel
bd74132
@basepi basepi marked this pull request as ready for review March 2, 2022 17:44
@basepi basepi requested a review from beniwohli March 2, 2022 17:44
@basepi basepi changed the title [WIP] opentelemetry bridge opentelemetry bridge Mar 2, 2022
@basepi basepi mentioned this pull request Mar 2, 2022
Open
beniwohli
beniwohli approved these changes Mar 7, 2022
View reviewed changes
Copy link
Contributor

@beniwohli beniwohli left a comment

Choose a reason for hiding this comment

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

Some minor comments, other than that LGTM!
@basepi basepi merged commit 6ddc155 into elastic:main Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent-python
3 participants
@basepi @beniwohli @houndci-bot