Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Telemetry events #341

Open
whatyouhide opened this issue Apr 3, 2024 · 6 comments
Open

Telemetry events #341

whatyouhide opened this issue Apr 3, 2024 · 6 comments

Comments

@whatyouhide
Copy link
Contributor

whatyouhide commented Apr 3, 2024

This issue is a starting point to discuss what the API for Telemetry events (emitted by Req) will look like.

An initial sketch would be to have default events like this, resembling a telemetry:span/3.

  • [:req, :request, :pipeline, :start]
  • [:req, :request, :pipeline, :stop]
  • [:req, :request, :pipeline, :exception]

These would measure the entire request pipeline.

I would love if we could also customize the prefix, something like this:

Req.new(
  # ...,
  telemetry: [
    prefix: [:my_app, :stripe_client]
  ]
)

Resulting in events such as [:my_app, :stripe_client, :pipeline, ...].

Steps

At some point we might want to instrument steps, such as JSON decoding/encoding, but I’m not sure this is critical to get this out the door. Adding Telemetry events is not a breaking change, so this is not a blocker for 1.0.

cc @wojtekmach

@whatyouhide
Copy link
Contributor Author

One important consideration: retries. We should think of how to instrument those, because they should not be instrumented like the whole request pipeline. Otherwise, you might end up with requests that look like they took seconds because they were retried a few times. Just putting this out there.

@yordis
Copy link

yordis commented Jun 20, 2024

I think you need at least two telemetry IMO,

@aloukissas
Copy link

May I add: we definitely need to have the url template with path_params as part of the metadata, so e.g. we can get useful metrics time series. I heavily use this in Tesla (with PathParams and KeepRequest middleware).

@wojtekmach
Copy link
Owner

Agreed, thank you for mentioning it here.

@aloukissas
Copy link

One important consideration: retries. We should think of how to instrument those, because they should not be instrumented like the whole request pipeline. Otherwise, you might end up with requests that look like they took seconds because they were retried a few times. Just putting this out there.

One good example I've seen is in the stripe hex lib, where the retry count is one of the metadata fields. Works pretty ok.

@whatyouhide
Copy link
Contributor Author

Yeah I think emitting events and having the retry information in the event itself would be enough for other libs to build stuff on top of them.

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

No branches or pull requests

4 participants