Dagster: Add sentry logging

[bnchrch]

Overview

1. Adds sentry
2. Creates a sentry decorator to capture exceptions from ops and assets

closes #28673

Notes for reviewers

1. Unfortuantely dagster eats any errors so we have to wrap the functions ourselves
2. Datadog is not an option until we move to an ec2 deployment

Before merge

In prod dagster set

• SENTRY_DSN="..."
• SENTRY_ENVIRONMENT="production"
• TRACES_SAMPLE_RATE=1.0

[pedroslopez]

Nice! Left some questions.

Also - we had said in the original issue that we wanted error rates, but I don't think this gets us that. Sentry does show some error rate stuff for transactions, but I'm not sure those will show up in the current setup. Adding sentry might still be useful though. Could you confirm what your thinking is around the error rates? Did you manage to see transactions in sentry?

[pedroslopez]

From reading this, it seems we're doing this to ignore the logger? What exactly are we quieting down?

If we don't ignore the logger does this not also help surface the errors and avoid having us to manually report the exceptions to sentry? (assuming the exceptions go to the logger)

[bnchrch]

Good callout

So essentially here we had a trade off.

If we use sentry out of the box there is one large issue

1. Dagster eats all the user code errors and doesnt let them make it to the sentry level hook

However if we work around this (like we did in this PR)

We end up in a situation where the dagster logger is reporting an exception for every asset in the graph that failed (very noisy)

Ontop of this the dagster logs dont have the sentry tags applied which makes grouping and fingerprinting a more difficult task

So this ignore is meant to reduce the noise, only log sentry errors weve caught and have applied tags to in one way or another.

The reason im comfortable enough with the leaky bucket to recommend it is we still have slack errors to fallback on

[pedroslopez]

If we can run multiple of these and exceptions can occur from outside the "asset or op context", I would suggest wrapping this in a sentry scope so we ensure the tags are only set while executing the relevant code. I guess this scope belongs in the decorator method below.

This prevents tags from showing up on unrelated errors, leading things to incorrectly show up when filtering or leading us down the wrong path when debugging/investigating errors.

There's an example with scopes here

airbyte/airbyte-ci/connectors/pipelines/pipelines/sentry_utils.py

Line 21 in a339622

with sentry_sdk.configure_scope() as scope:

[bnchrch]

Updated to use scopes

[pedroslopez]

i'm actually not sure if this is helpful for these - I have it disabled for now on airbyte-ci pipelines but 🤷🏼

Did you see a use for it?

[bnchrch]

The use generally is that Ill leave it set to 1.0 in production to get performance data and error rates.

We can change this to a bool, just figured Id leave it a more direct number

[pedroslopez]

Is this information better served as a context? https://docs.sentry.io/platforms/python/enriching-events/context/

[bnchrch]

I grabbed it from an example, I thought it was useful, however looking at the docs it seems like its more of a debug statement in case you forget to log context 🤔

Will remove

[pedroslopez]

Also! question: what does the release in sentry show up as when an error is caught?

[bnchrch]

@pedroslopez just wanted to follow up on the error reporting comment

Also - we had said in the original issue that we wanted error rates, but I don't think this gets us that. Sentry does show some error rate stuff for transactions, but I'm not sure those will show up in the current setup. Adding sentry might still be useful though. Could you confirm what your thinking is around the error rates? Did you manage to see transactions in sentry?

So the reason I went with Sentry here was because until we decide to move to a self hosted / hybrid dagster deployment we are unable to send logs directly to datadog (as we wont be able to spin up the datadog agent).

With this implementation though we should still get full application monitoring through sentry and get an error rate for each job

Heres one example: https://airbytehq.sentry.io/performance/summary/?project=4504087736090624&query=&referrer=performance-transaction-summary&statsPeriod=30d&transaction=generate_cloud_registry&unselectedSeries=p100%28%29&unselectedSeries=avg%28%29

You can also get a rough waterfall view / flame graph by individual asset
https://airbytehq.sentry.io/performance/sentry-dev-test:54520cc347b5451780e9dd8a7c5f308d/?project=4504087736090624&query=&referrer=performance-transaction-summary&statsPeriod=30d&transaction=generate_cloud_registry&unselectedSeries=p100%28%29&unselectedSeries=avg%28%29

However each asset in a job at this time still has a separate trace id which means we dont have any information directly related to average run time at the job level.

This is because sentry doesnt have an explicit/non hacky way of setting trace id at the transaction level manually yet.

Also! question: what does the release in sentry show up as when an error is caught?
it shows as the git sha currently

https://airbytehq.sentry.io/issues/4356903801/?project=4504087736090624&query=&referrer=issue-stream&statsPeriod=14d&stream_index=10

[pedroslopez]

Thanks for explaining- Let's do it!

[pedroslopez]

Is this not spammy? Could be a debug log level thing

[bnchrch]

Good call!