Add enrichment for user_agent.{name, version} #174
Conversation
| if s.userAgentName == "" && s.inferredUserAgentName != "" { | ||
| span.Attributes().PutStr(semconv27.AttributeUserAgentName, s.inferredUserAgentName) | ||
| } | ||
| if s.userAgentVersion == "" && s.inferredUserAgentVersion != "" { | ||
| span.Attributes().PutStr(semconv27.AttributeUserAgentVersion, s.inferredUserAgentVersion) | ||
| } |
There was a problem hiding this comment.
[For reviewers] I am setting the user_agent.{name, version} only if it is not already set.
| }(), | ||
| }, | ||
| { | ||
| name: "user_agent_parse_name_version", |
There was a problem hiding this comment.
nit: I'd postfix these with something like _span and _transaction. So e.g. user_agent_parse_name_version_transaction here and user_agent_parse_name_version_span below, instead of having them with the same name.
There was a problem hiding this comment.
They would be reported as TestElastic{Transaction, Span}Enrich/user_agent_parse_name_version, is that not enough or is it for when looking at the tests individually without the context of the main test?
There was a problem hiding this comment.
Yeah, you are right. It's good enough. It just took me a few seconds to realize what's actually the difference is in these new tests with the same name. But yeah, this is ok - if the test fails, the output is clear.
There was a problem hiding this comment.
Cool, I am thinking to simplify these tests with a method for default enrichments... currently the tests are getting a bit hard to read an unnecessarily verbose. Will do it later as a followup.
2 participants
Adds enrichment for
user_agent.{name, version}if it is missing. The attributes are derived fromuser_agent.original.Closes: #172