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

The direction attribute does not comply with the naming guidelines in the spec #3129

Closed
sebastien-rosset opened this issue Jan 23, 2023 · 6 comments
Labels
spec:metrics Related to the specification/metrics directory

Comments

@sebastien-rosset
Copy link
Contributor

sebastien-rosset commented Jan 23, 2023

What are you trying to achieve?

Either:

  1. Fix the name of the direction attribute so that it's compliant with the naming guidelines.
  2. Amend the naming guidelines in the spec to handle scenarios such as direction.

What did you expect to see?

I was expected the OpenTelemetry spec to be self-consistent with naming conventions, in particular that the direction name is hierarchical.

Additional context.

The spec states the instrument should have attributes for direction; oddly, plural is used, which implies there could be more than one attribute for direction, but without explaining why plural makes sense. Further, that sentence is not stating the attribute should be named direction, but elsewhere it was interpreted as a literal requirement, i.e., the name of this attribute IS direction.

io - an instrument that measures bidirectional data flow should be called entity.io and have attributes for direction. For example, system.network.io.

Using the name direction breaks some of the semantic rules listed in the spec:

  1. https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/semantic_conventions/README.md#general-guidelines and
  2. https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/common/attribute-naming.md#attribute-naming

Metric names and attributes exist within a single universe and a single hierarchy. Metric names and attributes MUST be considered within the universe of all existing metric names. When defining new metric names and attributes, consider the prior art of existing standard metrics and metrics from frameworks/libraries.

One issue is that another namespace could have a different concept of "direction" such as up, down, or East, West, left, right, etc. For example a vane anemometer could have a direction attribute which reports wind direction in degrees or cardinal direction. Perhaps for I/O the attribute should be named hw.io_direction.

In addition, the spec states:

Names SHOULD NOT coincide with namespaces. For example if service.instance.id is an attribute name then it is no longer valid to have an attribute named service.instance because service.instance is already a namespace. Because of this rule be careful when choosing names: every existing name prohibits existence of an equally named namespace in the future, and vice versa: any existing namespace prohibits existence of an equally named attribute key in the future.

Defining attributes with no name hierarchy has the side effect of preventing any future use of that word as a namespace.

@bertysentry
Copy link
Contributor

Semantic conventions for System, Runtime Environment and Process use these attributes without prefixes:

  • direction
  • state
  • cpu
  • type
  • device
  • mode
  • mountpoint
  • protocol
  • status
  • pool
  • daemon
  • action
  • gc

direction is notably used for the following metrics:

  • process.disk.io
  • process.network.io
  • system.paging.operations
  • system.disk.io
  • system.disk.operation_time
  • system.disk.merged
  • system.network.dropped
  • system.network.packets
  • system.network.errors
  • system.network.io

Semantic conventions for hardware simply follow the same pattern and ensure consistency for this attribute and its possible values, as recommended by the general guidelines for metrics semconv.

My advice is that we must update the specification for attributes naming. While it is critical to have unique attribute names for Resources, it should not be mandatory for metric attributes.

Metric attributes are defined in the scope of a given metric (in OpenTelemetry) and there is no possible conflict of 2 attributes with identical names from 2 different metrics.

Having direction attribute in process.disk.io, system.network.io and hw.network.io is never going to be a problem.

If we can avoid prefixing everything as if it were Java class names, it will make everybody's life much easier too, and save some storage space as well ;-)

@sebastien-rosset
Copy link
Contributor Author

Thanks. If that's the case, then yes the specification for attributes naming should be updated. This would apply to #3131 as well.

Given that the spec explicitly states attribute names must be hierarchical, one could wrongly assume metric attributes do not have name collisions across metrics. This could be a problem when exporting metrics and their attributes to an external database. The spec could explicitly state the names of metric attributes are unique within the scope of a metric.

@sebastien-rosset
Copy link
Contributor Author

Presumably the spec would also have to state something about naming conventions for attributes defined in the spec itself versus attributes defined by other organizations. Someone could create metric-level attributes with common words or contractions such as com, memory, mtu, buffer, etc, which may collide with future use of the same words in the spec but with different semantics.

@sebastien-rosset
Copy link
Contributor Author

sebastien-rosset commented Apr 1, 2023

Thank you for the clarification.

Metric attributes are defined in the scope of a given metric (in OpenTelemetry) and there is no possible conflict of 2 attributes with identical names from 2 different metrics.

I thought the specification allowed for extensibility? I.e., a per-metric attribute can be defined in the specification, and a vendor may also define custom attributes. At least this is how I understood the recommendation to use hierarchical names. A hierarchical name such as com.myCompany.foo.bar helps to avoid collisions, even if the spec adds new attributes in future versions of the spec.
If a vendor can create a per-metric attribute with a flat name, then I don't see how it's possible to avoid future collisions. For example, one may want to create custom attributes such as form_factor or port_type. In that case, maybe the vendor will be lucky, or maybe not.

Shouldn't there be a way to ensure there is no possible collision between custom attributes and standard attributes?

@dmitryax
Copy link
Member

dmitryax commented Jan 18, 2024

Looks like this issue is resolved now. direction was recently changed to have different prefixes depending on a use case: system.paging.direction, disk.io.direction, network.io.direction.

@svrnm
Copy link
Member

svrnm commented May 6, 2024

We feel that this has been addressed, if you feel differently, leave a comment and we can reopen the issue, @sebastien-rosset

@svrnm svrnm closed this as completed May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec:metrics Related to the specification/metrics directory
Projects
None yet
Development

No branches or pull requests

5 participants