feat: add way of defining an attribute from first class field #82
Conversation
aaron-steinfeld
requested review from
avinashkolluru,
buchi-busireddy and
tim-mwangi
as code owners
Codecov Report
@@ Coverage Diff @@
## main #82 +/- ##
=========================================
Coverage 80.71% 80.71%
Complexity 222 222
=========================================
Files 27 27
Lines 752 752
Branches 56 56
=========================================
Hits 607 607
Misses 98 98
Partials 47 47
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
This comment has been minimized.
This comment has been minimized.
| SourceField source_field = 3; | ||
| } | ||
|
|
||
| enum SourceField { |
There was a problem hiding this comment.
This cannot be a new projection definition?
There was a problem hiding this comment.
all attribute definitions eventually resolve down to data - so for example, an attribute like span.startTime needs to be able to specify it's source_field = SOURCE_FIELD_START_TIME while something like span.requestUri could already do source_path = "http.request.uri" (or whatever tag that would correspond to). The projections come in when we go a level up. So for example, let's say we introduce an alias for span.startTime called span.timestamp - that'd use an attribute_id type projection or span.duration might use an expression projection like MINUS(Span.endTime, Span.startTime). Does that make sense?
There was a problem hiding this comment.
By a level up you mean like in Pinot?
There was a problem hiding this comment.
not necessarily. When we read attributes using the attribute reader ( https://github.com/hypertrace/hypertrace-ingester/blob/main/hypertrace-trace-enricher/trace-reader/src/main/java/org/hypertrace/trace/reader/attributes/DefaultValueResolver.java ), it uses the definition and recursively resolves it. So taking parts of the example before, we could imagine something like (pseudocode for brevity):
{
id: span.startTime,
definition: {
source_field = SOURCE_FIELD_START_TIME
}
},
{
id: span.endTime,
definition: {
source_field = SOURCE_FIELD_END_TIME
}
},
{
id: span.duration,
definition: {
projection: {
expression: {
operator: MINUS,
arguments: [
{
attribute_id: span.startTime,
attribute_id: span.endTime
}
]
}
}
}
To resolve a projection, we have to first resolve its arguments - walking down the tree until we get to the leaves which are either literals, source_path (that is, tag keys), or the new feature added here, source_field - that is, first class fields on a span/trace. Without that resolution, any projections could not use values from fields like startTime, endTime, spanId, traceId etc.
There was a problem hiding this comment.
As to the pinot question, query service does not read source_path or source_field, it only reads projections (since those can be rewritten as new queries) .
There was a problem hiding this comment.
Ok. Totally overlooked source_path. Makes sense.
|
Updated deps first in #83 |
This comment has been minimized.
This comment has been minimized.
Description
We previously allowed defining an attribute based on its string key, but certain fields in a span and trace are treated as first class and only accessible through those concrete fields. This change adds support to the attribute definition schema to access those values.
Testing
Schema change only (but used this functionality as part of an e2e verification)
Checklist: