getsentry / sentry-ruby

Sentry SDK for Ruby
https://sentry.io/for/ruby
MIT License
926 stars 493 forks source link

Naming Conventions for Propagating Traces #2200

Closed smeubank closed 8 months ago

smeubank commented 9 months ago

Describe the idea We have controls in SDKs to influence to what endpoints tracing headers should be propagated. This allows users to control what is being traced, where data is being sent. This is because not all endpoints are functionally relevant for users to trace (3rd parties) and sometimes they just don't want to have distributed tracing.

Today ruby has this option documented, config.propagate_traces. While in JS for example there is tracePropagationTarget

These play a similar role but do different things, boolean vs array of regexes, ruby BE and JS FE. Should we have the same naming convention in Ruby as others?

Why do you think it's beneficial to most of the users This is really just a naming convention thing, to have it same or similar across SDKs

Possible implementation

Python has trace_propagation_targets as optional, and is probably a better model for how Ruby should behave.

natikgadzhi commented 8 months ago

Let me see if I understand this correctly:

It looks like we should add the new option, and soft-deprecate the old one.

Another gotcha: we should prepare and ship the documentation change at the same time.

How far off am I? If my understanding is more or less alright, I'm happy to put this together. /cc @sl0thentr0py

sl0thentr0py commented 8 months ago

we already have trace_propagation_targets https://github.com/getsentry/sentry-ruby/blob/c5889798c9b35c0a10f03ac071c6e289da151592/sentry-ruby/lib/sentry/configuration.rb#L272-L275 https://docs.sentry.io/platforms/ruby/usage/distributed-tracing/limiting-trace-propagation/

ruby was the only SDK that had propagate_traces as a boolean before, I think it's fine to keep it around we just check for both https://github.com/getsentry/sentry-ruby/blob/c5889798c9b35c0a10f03ac071c6e289da151592/sentry-ruby/lib/sentry/net/http.rb#L99-L100

if you really want we can deprecate and remove the boolean, but not really necessary.

sl0thentr0py commented 8 months ago

I added missing docs https://github.com/getsentry/sentry-docs/pull/8813 closing this