Skip to content
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

Decide if event.domain separately from event.name is necessary #2994

Closed
tigrannajaryan opened this issue Nov 30, 2022 · 12 comments · Fixed by #3749 or open-telemetry/semantic-conventions#473
Assignees
Labels
spec:logs Related to the specification/logs directory

Comments

@tigrannajaryan
Copy link
Member

We mentioned the possibility of merging event.domain and event.name into one attribute. Now that even.domain is no longer on the Scope having it is a separate attribute is no longer mandatory.

There are a few things to consider here:

  • Whether the combined attribute is good enough for use cases @tedsuo mentioned ("primary index").
  • If merged how do we define the formatting of the attribute value, e.g. should the value be prefixed by reverse FQDN? Can we also use plain prefixes like k8s.?
  • What other operations need to be performed on the domain and name values which are qualitatively easier/possible when the values are recorded as separate attributes and are harder/impossible with a single combined attribute?
@tigrannajaryan tigrannajaryan added the spec:logs Related to the specification/logs directory label Nov 30, 2022
@tigrannajaryan
Copy link
Member Author

@open-telemetry/specs-logs-approvers FYI.

@djaglowski
Copy link
Member

Whether the combined attribute is good enough for use cases

I'm only aware of the "primary index" use case, which it seems this combined attribute would be sufficient for. Does anyone know of any other cases we need to account for?

@djaglowski
Copy link
Member

If merged how do we define the formatting of the attribute value, e.g. should the value be prefixed by reverse FQDN? Can we also use plain prefixes like k8s.?

I think OTel should be able to define "official" prefixes within the specification repo, and we should encourage reverse FQDN for unofficial attributes (e.g. a 3rd party defining their own)

@tigrannajaryan
Copy link
Member Author

Whether the combined attribute is good enough for use cases

I'm only aware of the "primary index" use case, which it seems this combined attribute would be sufficient for. Does anyone know of any other cases we need to account for?

The "group by domain" query is one such example where having one attribute doesn't work unless it is possible to unambiguously split the one attribute into the domain and name.

@jkwatson
Copy link
Contributor

jkwatson commented Dec 6, 2022

My personal opinion: event domain is probably not necessary, especially for user-generated "business events" (which I honestly think is probably the #1 use-case for the event API)

@tigrannajaryan
Copy link
Member Author

Here is another standard that uses separate fields: https://schema.ocsf.io/classes/http_activity

They use 3 different fields (class_uid, category_uid, activity_id), where class_uid implies the category_uid.

Interestingly then they further compose these 3 into a single type_uid field.

@patrickhousley
Copy link
Contributor

Per conversation within the Event WG, we have decided to remove the domain field. The name field will contain namespace prefix followed by a dot and then the event name.

Example: browser.mouse.click

  • The namespace for this event is browser.mouse
  • The event name for this event is click

Please reference the WG meeting notes for 2023-10-27 for discussion points.

@scheler
Copy link
Contributor

scheler commented Oct 30, 2023

We, at Cisco/AppDynamics, are using event.domain in some browser instrumentations in production, so please allow us to process this change and plan the migration before closing this issue. Thanks.

@patrickhousley
Copy link
Contributor

@scheler Do you have a timeframe for which you will need to process this change?

@jack-berg
Copy link
Member

@scheler what do you propose in terms of migration? I think we need to decouple changes to the spec / semantic conventions from changes to APIs / instrumentation that produce event.domain. That's what's being done with the http semantic conventions: we've proceeded with breaking changes to the semantic conventions while instrumentations develop migration plans to help users with the transition. In a similar way, I think we should proceed with this change to the semantic conventions / spec and open issues with any instrumentation producing event.domain to roll out the change in an acceptable schedule.

@scheler
Copy link
Contributor

scheler commented Nov 7, 2023

@jack-berg we could proceed with this change in the spec. In order to submit the changes to the semantic conventions introducing namespaced event names, I want to wait till we have a decision on how we represent event fields, so that the semantic convention PRs could include them too. Changes to the Events API should be spec'd next, and then the instrumentation changes and the migration planning could happen after that.

@MSNev
Copy link
Contributor

MSNev commented Nov 10, 2023

We, at Cisco/AppDynamics, are using event.domain in some browser instrumentations in production, so please allow us to process this change and plan the migration before closing this issue. Thanks.

As a "generalization" for this situation, I think we could include (somewhere) details that (if) the event.domain is present it should be considered to be a "prefix" to the event.name followed by event.name

In terms of the other way around, how can a backend "determine" the domain (which I assume you will have), this is not so clear cut. as previously both the event.name and event.domain supported embedding . while now we are declaring that the namespace is everything up to the last . -- So either you will have to "take" everything up to the "first" . and consider that to be your (previous) domain, or you have a set of "known" domain names and match them (and if you are storing the event.name as a separate field, split this off when storing)

carlosalberto pushed a commit that referenced this issue Nov 28, 2023
Fixes #2994

## Changes

Merging the `domain` and `name` fields for events and modifying language
to refer to the first part of the name as `namespace`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec:logs Related to the specification/logs directory
Projects
Status: Done
8 participants