[contrib:redis] marking redis service as CACHE by default #97
Conversation
ufoot
force-pushed
the
christian/redisascache
branch
from
3093ccb
to
8e7e67f
Compare
lib/ddtrace/contrib/redis/patcher.rb
Outdated
| @@ -59,7 +59,7 @@ def patch_redis_client | |||
| end | |||
|
|
|||
| def initialize(*args) | |||
| pin = Datadog::Pin.new(SERVICE, app: 'redis', app_type: Datadog::Ext::AppTypes::CACHE) | |||
| pin = Datadog::Pin.new(SERVICE, app: 'redis', app_type: Datadog::Ext::AppTypes::DB) | |||
There was a problem hiding this comment.
did we choose when we should use the cache type?
There was a problem hiding this comment.
This should ultimately be the user to decide about. Redis can act as both, this is just a default value. Most of the "added value" of the patch here is: actually do use the value in the Pin and avoid redis spans being reported as "custom" (which is clearly not the case).
There was a problem hiding this comment.
Our "service type" still lack a real Spec, but for now let's just have it consistent with Python. So here, gtm!
lib/ddtrace/contrib/redis/patcher.rb
Outdated
| @@ -60,6 +61,9 @@ def patch_redis_client | |||
| def initialize(*args) | |||
| pin = Datadog::Pin.new(SERVICE, app: 'redis', app_type: Datadog::Ext::AppTypes::DB) | |||
| pin.onto(self) | |||
| if pin.tracer && pin.service && pin.app && pin.app_type | |||
There was a problem hiding this comment.
Do you really need to test for pin.app && pin.app_type?
There was a problem hiding this comment.
Since the pin is created just before and we explicitly set those, no. Good catch. Side note: tracer and service are very likely defined too, but I'd rather be protective, while empty app and app_type will just yield uninteresting results, nil tracer or service would cause an error.
Fix proved a little more complex than expected, previously, actually, the service meta were just reported as expected. Basically, one needs to actively call set_service_info for it to work.
ufoot
force-pushed
the
christian/redisascache
branch
from
32f54a7
to
98d96ae
Compare
No description provided.