Fix arq tests in POTel #3875
Fix arq tests in POTel #3875
Conversation
❌ 345 Tests Failed:
View the top 3 failed tests by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
sentry_sdk/integrations/arq.py
Outdated
| status_unset = ( | ||
| hasattr(span._otel_span, "status") | ||
| and span._otel_span.status.status_code == StatusCode.UNSET | ||
| ) |
There was a problem hiding this comment.
I don't like that we're accessing the underlying _otel_span here -- would prefer to have POTelSpan manage the proxying as much as possible. We could add a status property to POTelSpan that returns _otel_span.status and use that here.
There was a problem hiding this comment.
Yes you are right. I thought this is done in multiple integrations and I will fix it in another Pr. But I guess I misremembered.
Will fix it in this PR
There was a problem hiding this comment.
I have now implemented a status property. Have a look at the tests, because the Otel span statuses and the Sentry span status are different, the Sentry status one can get with the status property can be different from the one that has been set with set_status() before. And I default to "UNKNOWN_ERROR". Does this look ok to you?
There was a problem hiding this comment.
I think this makes sense.
| @status.setter | ||
| def status(self, value): | ||
| # type: (Optional[Any]) -> None | ||
| raise NotImplementedError("Use set_status instead") |
There was a problem hiding this comment.
Maybe let's just get rid of this altogether.
Make sure OK status is set, only when there has not been a error status set before.