-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[receiver/k8seventsreceiver] Support for semantic attributes #14474
Comments
Pinging code owners: @dmitryax. See Adding Labels via Comments if you do not have permissions to add labels yourself. |
Related to this we also can support the new Event semantic conventions -> https://github.com/open-telemetry/opentelemetry-specification/blob/main/semantic_conventions/logs/events.yaml Example draft PR: #14603 What I'm not sure about is the uniqueness of the
So to make it unique we need to add the namespace in? We could use that in And so we are conflicting with https://github.com/open-telemetry/oteps/blob/main/text/0202-events-and-logs-api.md#events-structure as Event name is not unique?
Maybe I'm misunderstanding something? Maybe @scheler you can give some insights? Aaand @dmitryax what do you think ? |
@povilasv I will work on modifying the spec language to allow for this. The attribute Separately, I've a question on Kubernetes Events for my understanding - is the namespace+name combination unique across clusters? that is, (cluster+namespace+name) is unique and not just (namespace+name)? |
This came up in open-telemetry/opentelemetry-collector-contrib#14474 where it was highlighted that just `name` is not sufficient to provide for uniqueness of a Kubernetes Event. The attribute `event.domain` is only meant to be a logical separation across different event systems, so it allows for similar looking names in different domains but with a completely different purpose. However, within each domain we don't have to have any uniqueness constraints on the `name` and leave it to whatever is idiomatic to the particular event system the domain represents.
Which kubernetes field are is proposed to be mapped to |
@jack-berg The PR associated with this issue is mapping the Kubernetes Event's name to |
Yes the proposal is to use Kubernetes Event.Name for A couple of examples of K8s events:
|
In the case of multiple K8s clusters, event.Name + event.Namespace might collide. So to make it unique you would have to do something like event.name = Cluster Name + Event.Name + Event.Namespace to make it unique |
In those examples the Kubernetes event name is The kubernetes event.name appears to be an identifier for a particular kubernetes resource, which is a different concept. For one, kubernetes event.name will be high cardinality. Although not yet explicitly stated in the spec, opentelemetry Looking at the examples you've shown there, I'd say an appropriate |
Thanks for your input @jack-berg :) It makes sense, in these couple of examples we would have: event.name = ReplicaSet.SuccessfulCreate Some more different events from my K8s cluster: event.name = Pod.FailedScheduling And their event yamls:
|
I think using However, before we do that I have a question. Why do we need to record The primary purpose of event.name/event.domain is to make sure event consumers can clearly differentiate a subset of events they are interested in. In the case of Kubernetes events we already record a number of Kubernetes-specific attributes, such as What other reasons are there? |
I agree. Reason seems the best candidate for
Probably just comply with the new Events API spec. |
Also I believe we need to put the full k8s event payload into the log body. Thats my understanding from reading the new OTel Events spec. This approach is taken in the new receiver proposed in #14185 which does a bit more than events collection. If we agree on the approach, we can probably replace the k8s events receiver with the new one going forward |
I thought this is to conform the definition of Event in OpenTelemetry. If these attributes are missing, then these messages would be termed as logs. So if the backends are okay receiving Kubernetes events as logs then we can omit these attributes. Btw, recording metadata.name for |
I think this gets at the question I was trying to ask in the SIG meeting today but maybe not expressing well. What is the purpose of the |
There is no blanket requirement that every LogRecord produced by Collector complies with Events API spec. Events API is a spec for events produced by OpenTelemetry API, it is not a requirement for the Collector (at least it is not in the current understanding of the spec). We may voluntarily follow if we believe we specifically want k8s events to have a similar shape, but it should not be the default assumption for all log record sources in the Collector. |
My understanding of the purpose of the
|
The receiver actually produces events not logs. So I was thinking that having |
Events are logs (mostly) :-) We need to be careful with extending Otel API concepts unnecessarily to the Collector. We introduced an Otel Events helper API that allows to create LogRecords with a specific shape easily (with For now I still do not understand why the data that is produced by k8s events receiver needs to match the shape of LogRecord that is produced by Otel Events API. What is the rationale for this? I have a feeling that we are trying to shoehorn the data produced by k8s events receiver into the shape that Otel Events API defined because there is a co-incidence of terminology (both have the word "events" in them). Also keep in mind that |
The backends can have different processing pipelines for logs and events, so the telemetry producers should decide on what they are emitting and freeze it.
For RUM events, we want to have expectation on the set of attributes for each value of |
Right, but why are deciding that k8s events are more likely to be processed similar to RUM events than similar to for example K8s pod logs collected by filelog using Helm Chart (which are plain LogRecords)? Again, I think we should not by default assume every new kind of log record becomes an event. If that is the case then we failed with defining why we needed both the Events API and the Logs API. In that case we shouldn't have a Logs API and we should have event.name and event.domain as hard-coded fields of the log data model instead of being semantic conventions. I disagree with this approach. To me the default is exactly the opposite: every new source of logs is a LogRecord. Only if there is a strong evidence that it precisely fits the Otel API's Event definition then it should be shaped as an Event with event.name attribute on LogRecord and event.domain attribute on the Scope. |
Shouldn't this be the case for every structured log record? You need to know how to interpret the data contained in the record. That can come from an implicit expectation that all records in a given log stream conform to a set of conventions or by an explicit indication on each record. Why would RUM events be any different than k8s events from this perspective?
k8s events are emitted in a different way by a different system than pod logs. Pod logs are application logs that may or may not have structure and may or may not be part of an event-driven system. k8s events are events emitted by actors in an event-driven system and have shapes that can be determined based on the name of the event.
I'm worrying that we have decided that RUM events are somehow special and need to have their own API created for them which will be named as if it is generic but then people will be told that it isn't and they shouldn't use it as if it were. In my view, every log record represents an event. Every structured log record may represent an Event. Most probably do, but
Why would this only apply to
That should be easily fixed. There has to be a scope in the |
OK, looks like some clarifications and guidance is due in the Logs spec. I will try to put together something and will link from here. |
The spec is currently not very clear about what is a log and what is an event. Here is a for example a discussion that shows that there are different ways to interpret what is written in the spec today: open-telemetry/opentelemetry-collector-contrib#14474 This PR attempts to clarify the spec by defining the vocabulary more strictly and by adding a relevant FAQ.
The spec is currently not very clear about what is a log and what is an event. Here is a for example a discussion that shows that there are different ways to interpret what is written in the spec today: open-telemetry/opentelemetry-collector-contrib#14474 This PR attempts to clarify the spec by defining the vocabulary more strictly and by adding a relevant FAQ.
The spec is currently not very clear about what is a log and what is an event. Here is a for example a discussion that shows that there are different ways to interpret what is written in the spec today: open-telemetry/opentelemetry-collector-contrib#14474 This PR attempts to clarify the spec by defining the vocabulary more strictly and by adding a relevant FAQ.
The spec is currently not very clear about what is a log and what is an event. Here is a for example a discussion that shows that there are different ways to interpret what is written in the spec today: open-telemetry/opentelemetry-collector-contrib#14474 This PR attempts to clarify the spec by defining the vocabulary more strictly and by adding a relevant FAQ.
The spec is currently not very clear about what is a log and what is an event. Here is a for example a discussion that shows that there are different ways to interpret what is written in the spec today: open-telemetry/opentelemetry-collector-contrib#14474 This PR attempts to clarify the spec by defining the vocabulary more strictly and by adding a relevant FAQ.
I submitted open-telemetry/opentelemetry-specification#2863, let's discuss. |
The spec is currently not very clear about what is a log and what is an event. Here is a for example a discussion that shows that there are different ways to interpret what is written in the spec today: open-telemetry/opentelemetry-collector-contrib#14474 This PR attempts to clarify the spec by defining the vocabulary more strictly and by adding a relevant FAQ.
…t.name attributes represent (#2848) * Remove language around uniqueness of event names within a domain This came up in open-telemetry/opentelemetry-collector-contrib#14474 where it was highlighted that just `name` is not sufficient to provide for uniqueness of a Kubernetes Event. The attribute `event.domain` is only meant to be a logical separation across different event systems, so it allows for similar looking names in different domains but with a completely different purpose. However, within each domain we don't have to have any uniqueness constraints on the `name` and leave it to whatever is idiomatic to the particular event system the domain represents.
…t.name attributes represent (#2848) * Remove language around uniqueness of event names within a domain This came up in open-telemetry/opentelemetry-collector-contrib#14474 where it was highlighted that just `name` is not sufficient to provide for uniqueness of a Kubernetes Event. The attribute `event.domain` is only meant to be a logical separation across different event systems, so it allows for similar looking names in different domains but with a completely different purpose. However, within each domain we don't have to have any uniqueness constraints on the `name` and leave it to whatever is idiomatic to the particular event system the domain represents.
This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping Pinging code owners:
See Adding Labels via Comments if you do not have permissions to add labels yourself. |
…t.name attributes represent (#2848) * Remove language around uniqueness of event names within a domain This came up in open-telemetry/opentelemetry-collector-contrib#14474 where it was highlighted that just `name` is not sufficient to provide for uniqueness of a Kubernetes Event. The attribute `event.domain` is only meant to be a logical separation across different event systems, so it allows for similar looking names in different domains but with a completely different purpose. However, within each domain we don't have to have any uniqueness constraints on the `name` and leave it to whatever is idiomatic to the particular event system the domain represents.
…t.name attributes represent (#2848) * Remove language around uniqueness of event names within a domain This came up in open-telemetry/opentelemetry-collector-contrib#14474 where it was highlighted that just `name` is not sufficient to provide for uniqueness of a Kubernetes Event. The attribute `event.domain` is only meant to be a logical separation across different event systems, so it allows for similar looking names in different domains but with a completely different purpose. However, within each domain we don't have to have any uniqueness constraints on the `name` and leave it to whatever is idiomatic to the particular event system the domain represents.
This issue has been closed as inactive because it has been stale for 120 days with no activity. |
…t.name attributes represent (#2848) * Remove language around uniqueness of event names within a domain This came up in open-telemetry/opentelemetry-collector-contrib#14474 where it was highlighted that just `name` is not sufficient to provide for uniqueness of a Kubernetes Event. The attribute `event.domain` is only meant to be a logical separation across different event systems, so it allows for similar looking names in different domains but with a completely different purpose. However, within each domain we don't have to have any uniqueness constraints on the `name` and leave it to whatever is idiomatic to the particular event system the domain represents.
Describe the issue you're reporting
Would be cool to make some refactorings in k8s event receiver :)
One thing I was thinking about is parsing
involvedObject
to Kubernetes Resource Attributes using https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/resource/semantic_conventions/k8s.mdFor example, this Event:
Would end up with the following Resource attributes:
Also, currently namespace is in Log attribute, not Resource -> https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/receiver/k8seventsreceiver/k8s_event_to_logdata.go#L77-L85
So I would like to move that to Resource :)
Thoughts @dmitryax ? :)
The text was updated successfully, but these errors were encountered: