small updates to Telemetry

[robuye]

Hi there, I added some telemetry around webhook handler and changed metadata included in all API requests. It's all just a few small changes to make monitoring for stripity-stripe users awesome out of the box 🙃

Webhooks are often important part of integration with Stripe so having telemetry in there is good idea imo.

API requests metadata contains some low cardinality fields that can be useful when visualising these metrics in other tools. For example using stripe.request.stop event we can infer metrics for RED method and also have visibility into individual Stripe API endpoints.

There are no breaking changes in here, only telemetry metadata is different (url field is gone). There is assumption that event object always contains type property, which is always a case to my knowledge and Stripe docs.

Here is example how it looks in PhoenixLiveDashboard:

In order to enable this visualisation I only needed to add few lines to telemetry.ex:

      # Stripe
      summary("stripe.request.stop.duration",
        tags: [:endpoint, :status],
        unit: {:native, :millisecond}
      ),
      summary("stripe.webhook.stop.duration",
        tags: [:event],
        unit: {:native, :millisecond}
      ),

Other systems such as prom_ex, telemetry_ui, otel, etc can be easily integrated too.

Hope you like it, happy to take in any and all feedback.

[yordis]

CI is failing btw

[robuye]

CI is failing btw

Same error happens when I reverted all my changes, it seems to be related to something else. I will try to debug it further.

[robuye]

Okay, all tests are passing now, I think I fixed original problem by setting SHELL env. I also noticed mix format is updating 2 files so I fixed that too and added format checking to CI job.

[robuye]

Hii, I updated names per your feedback, but not completely.

1) I didn't include all headers because they can contain sensitive information such as cookies or api tokens 2) I strip query params from url so things like email don't end up in logs 3) I didn't include response content length because it requires parsing :hackney headers using low level API, this would make code more coupled to hackney adapter and it didn't feel like something you'd want

I also moved setting metadata one level higher so the original function is less complicated. Just experimenting and trying to fit in with current code patterns.

Lemme know what you think, if it looks good this way I will squash this commit with the original.

[robuye]

Hey, I'm having second thoughts about the usefulness of this PR. Everything I added here can be easily instrumented client-side too, just by wrapping calls to stripity_stripe with :telemetry.span. I'm not sure it's worth the new complexity this code brings in, do you think this it is worth going forward with? If not I will close the PR, no problem here.

[yordis]

I am OK taking these changes; I left a few comments the last time, primarily around the name of the tags and the value of them.

[robuye]

Okay, I think I addressed earlier feedback in code or in GH and also left some followup questions for you, I'm not sure you noticed it 😅

[github-actions[bot]]

This pull request has been automatically marked as "stale:discard". If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize reviewing it yet. Your contribution is very much appreciated!.