Auto-inference of destination.service.resource #1898
Conversation
|
I started also implementing the addition of the creation of exit spans, as described in the spec, but I think it doesn't make a lot of sense in the Java agent, as we don't expose any API to add custom data. A new API for exit span creation will look something like |
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
Trends 🧪💚 Flaky test reportTests succeeded. Expand to view the summary
Test stats 🧪
|
The difference is that users don't need to remember setting resource fields or worry about which value to set in order for the span to show up in the service map. |
apm-agent-core/src/test/resources/json-specs/service_resource_inference.json
Show resolved
Hide resolved
Users will have to remember this new dedicated API and not the "default" one, and I think it's actually better to force them to think what the resource would be, instead of implicitly use |
|
As decided offline with @felixbarny - the exit span creation API is added. |
| { | ||
| "span": { | ||
| "exit": "true", | ||
| "type": "db", | ||
| "subtype": "elasticsearch", | ||
| "context": { | ||
| "db": { | ||
| "type": "elasticsearch" | ||
| }, | ||
| "http": { | ||
| "url": { | ||
| "host": "my-cluster.com", | ||
| "port": 9200 | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "inferred_resource": "elasticsearch", | ||
| "failure_message": "If `context.db` exists without `context.db.instance`, the subtype should be used, even if `context.http` exists" | ||
| }, |
There was a problem hiding this comment.
We have the following priorities when inferring resource:
- use
context.dbfirst if available (1) - then try to use
context.messagingif available (2) - then try to use
context.httpif available (3)
While the relative ordering or (1) vs (3) is tested with the Elasticsearch case, we don't have test that ensures that (2) comes before (3), even if having a mix of messaging and http is unlikely we'd better cover this corner case. Also, maybe having a case with all of them should default to using the db part.
There was a problem hiding this comment.
OK, I'll add a test for messaging combined with http, but I don't think combining db and messaging makes sense
What does this PR do?
Closing #1848
Checklist