[pkg/stanza] Adopt semantic convention for the log file path attribute #37210
Conversation
douglascamata
changed the title
recombine operatorrecombine operator
| @@ -17,7 +17,7 @@ The `recombine` operator combines consecutive logs into single logs based on sim | |||
| | `max_unmatched_batch_size` | 100 | The maximum number of consecutive entries that will be combined into a single entry before the match occurs (with `is_first_entry` or `is_last_entry`), e.g. `max_unmatched_batch_size=0` - all entries combined, `max_unmatched_batch_size=1` - all entries uncombined until the match occurs, `max_unmatched_batch_size=100` - entries combined into 100-entry-packages until the match occurs | | |||
| | `overwrite_with` | `newest` | Whether to use the fields from the `oldest` or the `newest` entry for all the fields that are not combined. | | |||
| | `force_flush_period` | `5s` | Flush timeout after which entries will be flushed aborting the wait for their sub parts to be merged with. | | |||
| | `source_identifier` | `$attributes["file.path"]` | The [field](../types/field.md) to separate one source of logs from others when combining them. | | |||
| | `source_identifier` | | The [field](../types/field.md) to separate one source of logs from others when combining them. | | |||
There was a problem hiding this comment.
The default is still attributes["file.path"] (though the $ should be removed).
| | `source_identifier` | | The [field](../types/field.md) to separate one source of logs from others when combining them. | | |
| | `source_identifier` | `attributes["file.path"]`. | The [field](../types/field.md) to separate one source of logs from others when combining them. | |
However, the default appears to be out of date with the semantic convention, which is log.file.path, so we should change the default in another PR.
The "DefaultSourceIdentifier" is the default value used when the field is not present on a log entry.
There was a problem hiding this comment.
Hummm, I see, @djaglowski. I think I got confused because of the warning in https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/pkg/stanza/operator/transformer/recombine/transformer.go#L119.
So the "entry" in this case means the log entry being processed, not the configuration of that recombine, right?
Isn't it weird that a log entry didn't have a log file path? 🤔
There was a problem hiding this comment.
Oh oh, fixing this would be a breaking change, right? Is this change desired right now? I'm open to do it.
There was a problem hiding this comment.
So the "entry" in this case means the log entry being processed, not the configuration of that recombine, right?
Exactly.
Oh oh, fixing this would be a breaking change, right? Is this change desired right now? I'm open to do it.
Yes, unfortunately, but I think it's one we ought to do. The intention was always that logs would be recombined by default according to the files they were read from. Unfortunately it seems we updated the attribute name in one place and not the other so it's likely not working as intended for many users. Arguably this is just a bug fix, but since it's been in place for so long I think we should mark it as breaking.
There was a problem hiding this comment.
@djaglowski I updated the PR given the outcome of our conversation, including its title. Please have a look whenever you have some time, thank you!
douglascamata
changed the title
recombine operator…tor-contrib into fix-recombine-operator-docs
|
Thanks for taking this on @douglascamata |
|
@djaglowski my pleasure. Thank you very much for the review! 🙇 I handled your feedback in c90a15c, so please have a look again when you can. |
…tor-contrib into fix-recombine-operator-docs
|
My rebase with |
…tor-contrib into fix-recombine-operator-docs
|
The build is fixed, @djaglowski |
|
Hey @djaglowski, please have another look at this when you have some time. Thanks! |
|
Thanks @douglascamata! |
open-telemetry#37210) #### Description This PR adopts [the semantic convention for the log file path attribute](https://github.com/open-telemetry/semantic-conventions/blob/main/docs/general/logs.md#log-file), which should be `attributes["log.file.path"]`. It fixes the default value for the `recombine` operator's `source_identifier`.
open-telemetry#37210) #### Description This PR adopts [the semantic convention for the log file path attribute](https://github.com/open-telemetry/semantic-conventions/blob/main/docs/general/logs.md#log-file), which should be `attributes["log.file.path"]`. It fixes the default value for the `recombine` operator's `source_identifier`.
Description
This PR adopts the semantic convention for the log file path attribute, which should be
attributes["log.file.path"].It fixes the default value for the
recombineoperator'ssource_identifier.