fix(datadog): only http.route for resource #810
Conversation
There was a problem hiding this comment.
Approved, but a quick caveat:
I was thinking through our discussion earlier and I wonder if http.target should be the fall-back?
Since every individual resource in datadog becomes it's own line in APM, I worry that you'll end up with some giant page of thousands (or millions) of resource links because you're using the target.
If this is how DD normally handles it, great. But in the OT world we explicitly call out that the default if "route" is not available for span name is just the HTTP method, since the cardinality is small:
| @@ -222,8 +222,8 @@ def test_export(self): | |||
| def test_resources(self): | |||
| test_attributes = [ | |||
| {}, | |||
| {"http.method": "GET", "http.route": "/foo"}, | |||
| {"http.method": "GET", "http.path": "/foo"}, | |||
| {"http.method": "GET", "http.route": "/foo/<int:id>"}, | |||
There was a problem hiding this comment.
might be good to add a test case with both http.route and http.target (a common scenario) and ensure that the precedence is correct?
There was a problem hiding this comment.
I went with the conservative approach you suggested, not backing off to http.target, being that it would create a cardinality issue.
|
@majorgreys can you take a look at comments? then we can merge. |
majorgreys
changed the title
…ry#810) * docs: add example for express plugin open-telemetry#800 * chore: add link to express example in plugin's readme * chore: addres PR's comments
…ry#810) * docs: add example for express plugin open-telemetry#800 * chore: add link to express example in plugin's readme * chore: addres PR's comments
Address an inconsistency with the the semantic conventions for HTTP the Datadog exporter uses to specify the HTTP route.