Skip to content
This repository has been archived by the owner on Sep 17, 2024. It is now read-only.

Add support for Open Telemetry #186

Open
rodolfoBee opened this issue Aug 10, 2023 · 5 comments
Open

Add support for Open Telemetry #186

rodolfoBee opened this issue Aug 10, 2023 · 5 comments

Comments

@rodolfoBee
Copy link
Member

Problem Statement

Our current Profiling solution relies on the SDK creating and finishing transactions on its own. When using open telemetry these actions are performed by otel, and Sentry simply collect otel spans and translate them into Sentry transactions and spans.

As a result, profiling data is not created by Sentry.

Solution Brainstorm

Add support for otel instrumentation.

@getlarge
Copy link

Hey @JonasBa, i’m pinging you as you seem to be contributing the most to this project.
Few questions:

  • Could you tell if there’s any plan to support profiling when using Otel instrumentation ?
  • Would this depend on profiling events being generated and sent by OpenTelemetry SDK (which seems to be WIP)?
  • Could profiling information be added to Transactions and Spans created by the @sentry/opentelemetry-node lib ?
    I haven’t had a look at the code from the profiling-node so that might be a naive suggestion, but maybe using the transaction events (either native from Sentry or from OTel SDKs) could be used as hooks to attached those data into transactions ?
  • Can i help you ?

@JonasBa
Copy link
Member

JonasBa commented Aug 28, 2023

@getlarge @rodolfoBee I need to talk to the team to see how this would work, afaik, there is no defined spec for Otel profiling, so I would start there. Is there some library which already integrates profiling with otel spec that you know of?

@getlarge
Copy link

getlarge commented Aug 31, 2023

There does not seem to be a profiling spec from OTel yet, only some agreed considerations on how to build it.
This project was mentioned in the original issue that proposed to add linking of profiles with traces, it is written in Go and uses pprof to create profiles.
There is no Node.JS lib equivalent, even though Pyroscope has a Node lib to create and upload profiles on their platform. It also uses pprof.

@JonasBa
Copy link
Member

JonasBa commented Aug 31, 2023

@getlarge Thanks for listing the links, I re-read them and it doesn't seem like much has changed in terms of outlining the concrete steps towards conforming to a standardized profiling format. My gut sense is that folks are slowly converging towards pprof which is not something Sentry currently supports (note that we currently do not ingest an aggregated call graph - we do this in order to be able to display the flamechart in chronological order while actual aggregation is performed by our processing pipelines).

We also have constraints related to our data model that we are looking to solve before we can support this. In short, profiles at Sentry are currently tied to transactions, meaning a profile cannot exist without a transaction or it is discarded at ingestion. Once we remove this constraint we can start looking into supporting standalone profiles as well as a continuous profiling approach.

The simplest way I see this working for sentry node profiling is if we align on using pprof, but before we can make that call we need verify the feasibility with the rest of our platforms (we dont want different platforms sending us different profiling formats). I think a lot of the work needed to support this will be done once we start working on continuous profiling, but I dont have a timeline for that right now.

@JonasBa
Copy link
Member

JonasBa commented Sep 11, 2023

@getlarge the otel profiling spec is shaping up, see open-telemetry/oteps#237

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants