Otel trace groups #2
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,216 @@ | ||
| { | ||
| "version": 1, | ||
| "template": { | ||
| "mappings": { | ||
| "date_detection": false, | ||
| "_source": { | ||
| "enabled": true | ||
| }, | ||
| "dynamic_templates": [ | ||
| { | ||
| "integer_resource_attributes": { | ||
|
There was a problem hiding this comment. shouldn't this be |
||
| "mapping": { | ||
| "type": "long" | ||
| }, | ||
| "path_match": "resource.attributes.*", | ||
| "match_mapping_type": "long" | ||
| } | ||
| }, | ||
| { | ||
| "double_resource_attributes": { | ||
| "mapping": { | ||
| "type": "double" | ||
| }, | ||
| "path_match": "resource.attributes.*", | ||
| "match_mapping_type": "double" | ||
| } | ||
| }, | ||
| { | ||
| "default_resource_attributes": { | ||
|
There was a problem hiding this comment. @KarstenSchnitter Is this correct? Default attributes can be objects also, right? It looks like, here you are assuming them to be strings. I think it is possible for attributes to be nested like There was a problem hiding this comment. Due to the There was a problem hiding this comment. @KarstenSchnitter I am seeing this error with the mappings you provided - Any idea why this error is occurring? The mappings from the server There was a problem hiding this comment. I have deleted the "default_resource_attributes", "default_scope_attributes" and "default_attributes" and then I was able to store the data and see the data using opensearch dashboards. There was a problem hiding this comment. This looks like a typical type mismatch in the error message. Note, that the There was a problem hiding this comment. @KarstenSchnitter sorry, that was copy-paste mistake. Even with the correct mappings with "keyword", I am still seeing the error. For now, I am going to take out the default attributes section in all three and updated the PR. These changes are working fine for traces,logs and metrics. Please submit another PR separately (on top of this) for any changes you want to make after I merge these change. There was a problem hiding this comment. Please do not take it out. Mapping to keywords is the essential default we need. Can you try with an additional |
||
| "mapping": { | ||
| "type": "keyword", | ||
| "ignore_above": 256 | ||
| }, | ||
| "path_match": "resource.attributes.*" | ||
| } | ||
| }, | ||
| { | ||
| "integer_scope_attributes": { | ||
| "mapping": { | ||
| "type": "long" | ||
| }, | ||
| "path_match": "instrumentationScope.attributes.*", | ||
| "match_mapping_type": "long" | ||
| } | ||
| }, | ||
| { | ||
| "double_scope_attributes": { | ||
| "mapping": { | ||
| "type": "double" | ||
| }, | ||
| "path_match": "instrumentationScope.attributes.*", | ||
| "match_mapping_type": "double" | ||
| } | ||
| }, | ||
| { | ||
| "default_scope_attributes_map": { | ||
| "mapping": { | ||
| "type": "keyword", | ||
| "ignore_above": 256 | ||
| }, | ||
| "path_match": "instrumentationScope.attributes.*" | ||
| } | ||
| }, | ||
| { | ||
| "long_attributes": { | ||
| "mapping": { | ||
| "type": "long" | ||
| }, | ||
| "path_match": "attributes.*" | ||
| } | ||
| }, | ||
| { | ||
| "double_attributes": { | ||
| "mapping": { | ||
| "type": "double" | ||
| }, | ||
| "path_match": "attributes.*" | ||
| } | ||
| }, | ||
| { | ||
| "default_attributes": { | ||
|
There was a problem hiding this comment. Same comment here. General attributes may also have nested attributes, right? |
||
| "mapping": { | ||
| "type": "keyword", | ||
| "ignore_above": 256 | ||
| }, | ||
| "path_match": "attributes.*" | ||
| } | ||
| } | ||
| ], | ||
| "properties": { | ||
| "droppedEventsCount": { | ||
| "type": "integer" | ||
| }, | ||
| "instrumentationScope": { | ||
| "properties": { | ||
| "droppedEventsCount": { | ||
| "type": "integer" | ||
|
There was a problem hiding this comment. Should this be There was a problem hiding this comment. Integer is at 32bits but signed. The proto definition specifies an unsigned 32bit integer. Do you think this difference is significant? I find it hard to imagine there would be a scenario where that number of attributes is ever attached and dropped to a log/metric/span. There was a problem hiding this comment. You are just referring to the naming, right? I am going to correct that. |
||
| }, | ||
| "schemaUrl": { | ||
| "type": "keyword", | ||
| "ignore_above": 256 | ||
| }, | ||
| "name": { | ||
| "type": "keyword", | ||
| "ignore_above": 128 | ||
| }, | ||
| "version": { | ||
| "type": "keyword", | ||
| "ignore_above": 64 | ||
| } | ||
| } | ||
| }, | ||
| "resource": { | ||
| "properties": { | ||
| "droppedEventsCount": { | ||
|
There was a problem hiding this comment. shouldn't this be There was a problem hiding this comment. I am confused. What do want to change? This is the name of the field. Only indentation seems to be off. There was a problem hiding this comment. I am going to change this to |
||
| "type": "integer" | ||
| }, | ||
| "schemaUrl": { | ||
| "type": "keyword", | ||
| "ignore_above": 256 | ||
| } | ||
| } | ||
| }, | ||
| "severity": { | ||
| "properties": { | ||
| "number": { | ||
| "type": "integer" | ||
| }, | ||
| "text": { | ||
| "type": "keyword", | ||
| "ignore_above": "32" | ||
| } | ||
| } | ||
| }, | ||
| "body": { | ||
| "type": "text" | ||
| }, | ||
| "@timestamp": { | ||
| "type": "date_nanos" | ||
| }, | ||
| "time": { | ||
| "type": "date_nanos" | ||
| }, | ||
| "observedTimestamp": { | ||
| "type": "date_nanos" | ||
| }, | ||
| "observedTime": { | ||
| "type": "alias", | ||
| "path": "observedTimestamp" | ||
| }, | ||
| "traceId": { | ||
| "type": "keyword", | ||
| "ignore_above": 32 | ||
| }, | ||
| "spanId": { | ||
| "type": "keyword", | ||
| "ignore_above": 16 | ||
|
There was a problem hiding this comment. Why are we limiting to 16 chars here and 32 in traceId? There was a problem hiding this comment. First, this is not a limitation of storage, just of the indexing. W3C Trace Context specifies the trace id to be an array of 16 bytes and the span id to be an array of 8 bytes. With hex-encoding this produces 32 and 16 characters respectively. Using B3 context would provide the same length. So it seems to be a reasonable choice. |
||
| }, | ||
| "flags": { | ||
| "type": "long" | ||
|
There was a problem hiding this comment. https://github.com/open-telemetry/opentelemetry-proto/blob/main/opentelemetry/proto/logs/v1/logs.proto#L178 There was a problem hiding this comment. This is defined as There was a problem hiding this comment. I am going to check, that for very attribute group there is a dropped attributes count. Thanks for pointing this out. There was a problem hiding this comment. I renamed all the |
||
| }, | ||
| "schemaUrl": { | ||
| "type": "keyword", | ||
| "ignore_above": 256 | ||
| }, | ||
| "serviceName": { | ||
| "type": "alias", | ||
| "path": "resource.attributes.service.name" | ||
| }, | ||
| "event": { | ||
|
There was a problem hiding this comment. Why do we need event here? There was a problem hiding this comment. That is a question I am wondering myself. This comes out of the Simple Schema for Observability. I kept it, since it was in the original file proposed by @kkondaka. I know this is not in the OpenTelemetry standard. The same is true for the There was a problem hiding this comment. Ok, I take it out in my PR to your branch. |
||
| "properties": { | ||
| "kind": { | ||
| "type": "keyword", | ||
| "ignore_above": 256 | ||
| }, | ||
| "domain": { | ||
| "type": "keyword", | ||
| "ignore_above": 256 | ||
| }, | ||
| "category": { | ||
| "type": "keyword", | ||
| "ignore_above": 256 | ||
| }, | ||
| "type": { | ||
| "type": "keyword", | ||
| "ignore_above": 256 | ||
| }, | ||
| "result": { | ||
| "type": "keyword", | ||
| "ignore_above": 256 | ||
| }, | ||
| "exception": { | ||
| "properties": { | ||
| "message": { | ||
| "type": "text", | ||
| "fields": { | ||
| "keyword": { | ||
| "type": "keyword", | ||
| "ignore_above": 2048 | ||
| } | ||
| } | ||
| }, | ||
| "stacktrace": { | ||
| "type": "text" | ||
|
There was a problem hiding this comment. There was a problem hiding this comment. This was in the original file. The OTel specification only guarantees, that at least one of There was a problem hiding this comment. I think There was a problem hiding this comment.
|
||
| }, | ||
| "type": { | ||
| "type": "keyword" | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you also add eventName which is missing: https://github.com/open-telemetry/opentelemetry-proto/blob/main/opentelemetry/proto/logs/v1/logs.proto#L226
We have done some analysis on logs mapping here: opensearch-project/opensearch-catalog#217
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will add the field. I am going to take a look at the analysis.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@KarstenSchnitter It looks like we need to bump up the version number to 3 (at least for spans). Otherwise new template mappings are not being sent to the OpenSearch server.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where does this limitation come from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have we added this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, this could look much nicer, if I could use component templates. That way, I could use the same dynamic mappings on all three templates. Is there a way to do that with Data Prepper?