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

Document in SDK Expected Features the HTTP integrations #254

Closed
bruno-garcia opened this issue Jan 25, 2021 · 12 comments · Fixed by #341
Closed

Document in SDK Expected Features the HTTP integrations #254

bruno-garcia opened this issue Jan 25, 2021 · 12 comments · Fixed by #341

Comments

@bruno-garcia
Copy link
Member

bruno-garcia commented Jan 25, 2021

In SDK Expected Features we need some guideline of what we expect from integrating with HTTP clients in SDKs, when that's possible.

For example:

  • Add a breadcrumb to the Scope (so error events show that request)
    • Mention the breadcrumb categories and types that make sense here. Include HTTP status.
    • category could be the name of the HTTP integration. Type is http. Docs
  • Get some metrics (that for now can go as breadcrumb.data Example: bytes In, Bytes Out).
  • Should the SDK raise an event if a 5xx comes? If so what contexts.. If not, how does it propagate context upstream?

If Performance Monitoring is supported by the SDK:

  • Create a Span, child to the current Span bound to the Hub
  • Finish the span as success or failure depending on response HTTP status (500 is a failed Span)
    • We need to align/discuss 4xx and other status code range.
  • Include sentry-trace id to propagate a transaction downstream. Link to specific docs.

Link to some examples:

.NET
Java
Python

@marandaneto
Copy link
Contributor

@maciejwalkowiak after finishing the PR and docs, let's tackle this.

@marandaneto marandaneto removed their assignment Mar 17, 2021
@bruno-garcia
Copy link
Member Author

@kamilogorek how does this work for XHR and fetch?

@kamilogorek
Copy link
Contributor

What are you asking about exactly? We do capture breadcrumbs for these, as well as spans when tracing is enabled.

@marandaneto
Copy link
Contributor

@kamilogorek I guess its a general question about what do you do when instrumenting HTTP requests?

Start/Finish a span, also include sentry-trace header
Add a breadcrumb
Capture an error if any, do we collect metrics, etc

@marandaneto
Copy link
Contributor

@kamilogorek any input here? thanks

@kamilogorek
Copy link
Contributor

Uh, sorry, you could ping me way earlier 🥲

For both, XHR and Fetch we:

  • capture span (if tracing enabled - requires @sentry/tracing package)
  • include sentry-trace (if tracing enabled - requires @sentry/tracing package)
  • capture breadcrumbs (success status, response code, url, method - through integration, turned on by default)
  • no additional metrics captured (eg. timings)

@marandaneto
Copy link
Contributor

marandaneto commented Apr 15, 2021

@kamilogorek forgot about it too ^^

do you captureException(x) if the request throws for some reason? or you let the user to handle that

@kamilogorek
Copy link
Contributor

We let it bubble up to global error handlers and handle it there.

@rhcarvalho
Copy link
Contributor

We let it bubble up to global error handlers and handle it there.

@marandaneto One thing to pay attention here is what the user expectation is and what the product does. Right now we have a problem that Fetch errors end up linked to the transaction instead of the specific span. Ideally the specific span where an error happened should be the one providing the span context for the error event.

@marandaneto
Copy link
Contributor

yep, thats what I've thought as the span is already finished.

@rhcarvalho what we do is to keep a WeakRef<Span, Exception> of the Exception, see the okhttp https://github.com/getsentry/sentry-java/blob/main/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt#L35
so if the event gets captured by the user or handlers, the event will contain the correct span info.

@lobsterkatie
Copy link
Member

Jus throwing an asterisk in here that once the dynamic sampling stuff goes live, anything about sentry-trace will also be about tracestate, and will therefore involve different methods (getTraceHeaders rather than toSentryTrace, for example). Nothing to do right now, just something to keep in the back of our collective minds.

@marandaneto
Copy link
Contributor

Should the SDK raise an event if a 5xx comes? If so what contexts.. If not, how does it propagate context upstream?

is missing but as soon as we get some feedback from the Dart integration that also captures events from failed requests, @ueman will include that too.

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

Successfully merging a pull request may close this issue.

5 participants