-
Notifications
You must be signed in to change notification settings - Fork 375
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
ActiveSupport::Notifications subscriptions #380
ActiveSupport::Notifications subscriptions #380
Conversation
ce012bf
to
cce5bed
Compare
@dasch would you like to review this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
cce5bed
to
cce6a81
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we add a new functionality without altering the rest of the tracer, we can safely add it in the next minor release.
@delner I feel like this is a stupid question, but how would one go about testing this really quick without getting to deep into ActiveSupport::Notifications.subscribe(FastJsonapi::ObjectSerializer::SERIALIZABLE_HASH_NOTIFICATION) do |name, start, finish, id, payload|
# send to DD
end Would I just do something like this in the block? span = Datadog.tracer.trace('my-trace')
span.resource = payload[:name]
span.start_time = start
span.finish(finish) |
@rromanchuk You could do something like that in the block, you would basically be making a span retroactively after a finished event. Just be sure to set a service, too. This subscription pattern has matured a bit since we first introduced it. We made an A simple implementation of Event this might look something like: class SerializableHashEvent
include ActiveSupport::Notifications::Event
def event_name
FastJsonapi::ObjectSerializer::SERIALIZABLE_HASH_NOTIFICATION
end
def span_name
'my-trace'
end
def process(span, event, id, payload)
# You can add tags or modify the span here.
end
end
SerializableHashEvent.subscribe! |
@delner cool thanks for the guidance. I'll eventually take a look how these services were implemented and clean this up instead of monkey patching with an initializer. Ideally, submit a datadog integration directly to
If i'm also using the the rails integration On that note, although i'm still in the process of learning as i'm setting up my integration, i find myself struggling with best practices on naming conventions, but I think it will become more clear once I start interacting with APM, in this case i'm assuming i want my service name to be identical to my rails app as this is really just the last mile of my request? This is where i'm at ActiveSupport::Notifications.subscribe(FastJsonapi::ObjectSerializer::SERIALIZABLE_HASH_NOTIFICATION) do |name, start, finish, id, payload|
# name is render.fast_jsonapi.serializable_hash
# payload is {:name=>"MySerializer"}
span = Datadog.tracer.trace(name)
span.service = "MyRailsApp"
span.resource = payload[:name]
span.span_type = "http"
span.start_time = start
span.finish(finish)
end |
You still need to set service via Looking at other integrations will likely be the best way of understanding the conventions, but as a really short cheatsheet:
Given those, I'd recommend changing your service name a bit, maybe give it a service name like |
Thanks that helps. It immediately became clear to me i wanted a unique service name the second i looked at the flame graph. Looks like i need to clean up the rest of my service names too, makes more sense using it in practice. I was conflating the entire HTTP API request as a single service, which I guess technically could be desired depending on the scope. Looks like I accidentally introduced duplicate spans. I've got One last question i promise. I realized i had APM configuration settings in my ansible deploy for my datadog role, but i completely forgot why and how I added them. Looks like some type of filter, can i completely remove this? Maybe it's just legacy, because i don't see any usage or examples in https://github.com/DataDog/datadog-agent/blob/master/pkg/config/config_template.yaml apm_config:
enabled: true
analyzed_spans:
grape|grape.endpoint_run: 1
sparcore|rack.request: 1
net/http|http.request: 1
aws|aws.command: 1 |
There are numerous integrations that depend on ActiveSupport::Notifications to perform tracing. e.g. ActiveRecord, Racecar, Sequel, etc. Currently each of these integrations have their own implementations.
In the interest of creating some consistency, and encouraging new integrations that require ActiveSupport::Notifications, this pull request adds a
Datadog::Contrib::ActiveSupport::Notifications::Subscriber
module, that can be composed intoPatcher
s or other components that want to instrument ASN events with Datadog tracing.(This pull request is an updated version of #324; thanks @dasch for his contribution!)
A hypothetical ActiveRecord example (specific patterns, payloads may not be totally accurate):
Also allows for lazy subscriptions (useful if you want to trace two different events the same way):
Also see #381 for an example of an actual integration using this.