Skip to content

Support for Traces/Transactions via Opentelemetry#784

Closed
solnic wants to merge 53 commits into
masterfrom
solnic/support-for-transactions-via-otel
Closed

Support for Traces/Transactions via Opentelemetry#784
solnic wants to merge 53 commits into
masterfrom
solnic/support-for-transactions-via-otel

Conversation

@solnic

@solnic solnic commented Sep 4, 2024 •
edited
Loading

Copy link
Copy Markdown
Collaborator

This adds support for Traces via Opentelemetry integration, handled by our custom Sentry.Opentelemetry.SpanProcessor which accumulates spans and turns them into hierarchical transactions that are then sent to Sentry.

For this to work the following deps must be added to an app:

{:opentelemetry, "~> 1.4"},
{:opentelemetry_api, "~> 1.3"}

Furthermore, Opentelemetry needs to be configured with our SpanProcessor:

config :opentelemetry, span_processor: {Sentry.Opentelemetry.SpanProcessor, []}

The rest will automagically work.

Supported opentelemetry packages

This initial implementation supports the following Opentelemetry packages:

  • opentelemetry_phoenix
  • opentelemetry_bandit
  • opentelemetry_ecto
  • opentelemetry_oban

Considerations

  • Should we add new deps as sentry deps instead of having people add them manually to their mix.exs?
  • Would it make sense to try to pre-configure opentelemetry automatically?
  • Which other opentelemetry_* packages do we want to support initially?
  • Should we support top-level ecto events that occur when ie Ecto tries to create migrations table, or checks migrations? (I think we should ignore them, FWIW)

solnic
solnic commented Sep 5, 2024
View reviewed changes
Comment thread test/sentry/telemetry/span_processor_test.exs Outdated
@solnic solnic force-pushed the solnic/support-for-transactions-via-otel branch 5 times, most recently from 6f8f1ea to 1478508 Compare September 20, 2024 10:12
@solnic solnic force-pushed the solnic/support-for-transactions-via-otel branch 4 times, most recently from e8a40bb to cb77932 Compare September 27, 2024 13:16
@solnic solnic changed the title [WIP] Support for Transactions/Spans via Opentelemetry Span Processor Support for Transactions/Spans via Opentelemetry Span Processor Sep 27, 2024
@solnic solnic marked this pull request as ready for review September 27, 2024 14:16
@solnic solnic changed the title Support for Transactions/Spans via Opentelemetry Span Processor Support for Traces via Opentelemetry Sep 27, 2024
@solnic solnic changed the title Support for Traces via Opentelemetry Support for Traces/Transactions via Opentelemetry Sep 27, 2024
@solnic solnic mentioned this pull request Oct 8, 2024
Closed
@solnic solnic force-pushed the solnic/support-for-transactions-via-otel branch from cb77932 to 93579f3 Compare October 14, 2024 10:05
@solnic solnic mentioned this pull request Oct 14, 2024
Merged
@whatyouhide whatyouhide added this to the 10.8.0 milestone Oct 22, 2024
@whatyouhide

whatyouhide commented Oct 22, 2024

Copy link
Copy Markdown
Collaborator

@solnic can you rebase this one now that #797 has been merged? Gonna make it easier to review I think 😄

@whatyouhide whatyouhide mentioned this pull request Oct 22, 2024
Open
@solnic solnic force-pushed the solnic/support-for-transactions-via-otel branch 2 times, most recently from 87fe610 to 563e416 Compare October 23, 2024 09:16
@solnic

solnic commented Oct 23, 2024

Copy link
Copy Markdown
Collaborator Author

@solnic can you rebase this one now that #797 has been merged? Gonna make it easier to review I think 😄

@whatyouhide done!

solnic
solnic commented Oct 23, 2024
View reviewed changes
Comment thread .github/workflows/main.yml Outdated
whatyouhide
whatyouhide requested changes Oct 23, 2024
View reviewed changes
Comment thread .github/workflows/main.yml Outdated
Comment thread lib/sentry.ex Outdated
Comment thread lib/sentry/opentelemetry/span_processor.ex Outdated
Comment thread lib/sentry/opentelemetry/span_storage.ex Outdated
Comment thread mix.exs Outdated
@solnic solnic force-pushed the solnic/support-for-transactions-via-otel branch from 08cec4c to e77a601 Compare October 28, 2024 09:15
@whatyouhide whatyouhide mentioned this pull request Oct 28, 2024
Merged
solnic and others added 18 commits December 27, 2024 11:57
@solnic
Fix tests under 1.13
6788ad5 
Turns out `optional` deps in mix.exs work differently in
older Elixir, as it would not load opentelemetry deps which
made our tests fail because opentelemetry setup was
not loaded.
@solnic
Support Transaction in client reports
7b2fc02
@solnic
wip - initial work on oban support
4f3ada1
@solnic
wip - add a UI for testing Oban workers
1856faa
@danschultzer @solnic
Refactor OTel (#835)
6c2d80c 
* Add environment to transaction

* Make root span the transaction

* Generic transactions spans parsing
@solnic
Update dep specs for opentelemetry_*
b7b1dc6
@solnic
[tmp] use opentelemetry_oban from git
10224eb
@solnic
Fix warning in get_op_description
f67446e
@solnic
Fix formatting
4a175a9
@solnic
Refine how we set values for Oban jobs
d2f88a2 
- Use the worker name as the description and transaction name instead of "{worker} process"
- Use "queue.process" as the op - this is according to the docs https://develop.sentry.dev/sdk/telemetry/traces/modules/queues/

Maybe this could be improved in the opentelemetry_oban package?
@solnic
Extend the test worker UI with auto-scheduling
469b5cd
@solnic
Filter out Oban internal traces
9c59d36
@solnic
Oops, fix filtering Oban Stager spans
0832054
@solnic
Address dialyzer warning (I think)
01fcd87
@solnic @danschultzer
Set sdk version dynamically
4c32d87 
Co-authored-by: Dan Schultzer <1254724+danschultzer@users.noreply.github.com>
@solnic
Fix client report data category for transactions
9b4d700
@solnic
Add Sentry's Sampler for OTel
7ad38bb
@solnic
Remove conditional we longer need
a808377
@solnic solnic force-pushed the solnic/support-for-transactions-via-otel branch from 8574f4c to a808377 Compare December 27, 2024 11:58
@solnic solnic mentioned this pull request Dec 27, 2024
Closed
@solnic

solnic commented Dec 27, 2024

Copy link
Copy Markdown
Collaborator Author

I've split this into #842 and #843 /cc @whatyouhide

@solnic solnic mentioned this pull request Dec 27, 2024
Merged
@grantwest

grantwest commented Jan 4, 2025

Copy link
Copy Markdown

I've split this into #842 and #843 /cc @whatyouhide

Does that mean this PR will be closed without merging?

@solnic

solnic commented Jan 7, 2025

Copy link
Copy Markdown
Collaborator Author

I've split this into #842 and #843 /cc @whatyouhide

Does that mean this PR will be closed without merging?

Depending on which one is simpler to review, to be honest. I can of course rework this one on top of #842 but this is the 50th comment here so maybe we'd benefit from a cleaner slate. WDYT @whatyouhide?

@solnic solnic mentioned this pull request Jan 27, 2025
Closed
6 tasks
@jwaldrip

jwaldrip commented Feb 20, 2025

Copy link
Copy Markdown

Exciting to see progress. Been waiting a long time and can't wait to see the fruits of your labor!

@hugobarauna

hugobarauna commented Mar 6, 2025

Copy link
Copy Markdown

@solnic any updates here? This would help us in Livebook Teams. :)

@solnic

solnic commented Mar 14, 2025

Copy link
Copy Markdown
Collaborator Author

Closing this in favor of #875

@solnic solnic closed this Mar 14, 2025
@solnic solnic deleted the solnic/support-for-transactions-via-otel branch February 13, 2026 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@whatyouhide whatyouhide whatyouhide requested changes

@sl0thentr0py sl0thentr0py Awaiting requested review from sl0thentr0py

@savhappy savhappy Awaiting requested review from savhappy

+2 more reviewers

@danschultzer danschultzer danschultzer left review comments

@joaothallis joaothallis joaothallis left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

11.0.0

Development

Successfully merging this pull request may close these issues.

9 participants

@solnic @whatyouhide @yordis @danschultzer @grantwest @jwaldrip @hugobarauna @joaothallis @savhappy