Fix agent_url in environment logger when io transport #2313
Conversation
| def agent_url | ||
| # Retrieve the effect agent URL, regardless of how it was configured | ||
| transport = Tracing.send(:tracer).writer.transport | ||
|
|
||
| # return '' with IO transport | ||
| return '' unless transport.respond_to?(:client) | ||
|
|
||
| adapter = transport.client.api.adapter | ||
| adapter.url | ||
| end |
There was a problem hiding this comment.
I'm... not the biggest of fans of this approach. The issue showed up because we reach quite deep inside the transport object to get some information, and our "reaching" ended up being problematic. Let me inline it all so we can see how deep it is:
Datadog::Tracing.send(:tracer).writer.transport.client.api.adapter.url
This code is assuming a very very specific transport object, and will break with all others.
I feel that by probing for respond_to?(:client) we're solving the symptom (oops we need to be slightly more careful when reaching) and not the cause (we're reaching quite deep).
I won't block the PR on this (so I'll approve it anyway) BUT my suggestion would be to add an #agent_url method directly on the Datadog::Transport::Traces::Transport class.
I would also suggest having a useful fallback for others. E.g., if there's no agent url, perhaps we could instead use "(custom transport: #{transport.class})" or something like that.
Since the whole objective of this block is having something useful when reporting issues, I think having a clear indication that there's a non-standard config being used is quite valuable, instead of having it blank and whoever is looking at the issue will need to hunt around for a while to understand why.
What does this PR do?
Mitigate the error from environment logger when IO transport is configured.
The environment logger, seems to be only working for HTTP transport, the object has client attribute, and api and adapter so on…, but not for IO transport