open-telemetry / semantic-conventions

Defines standards for generating consistent, accessible telemetry across a variety of domains
Apache License 2.0
256 stars 165 forks source link

Proposal: Decoupling Attribute definition from Attribute usage in models #197

Open AlexanderWert opened 1 year ago

AlexanderWert commented 1 year ago

Context

Currently, in the model/** directory we mix up two things:

For example http.request.method_original is originally defined in model/trace/http.yml but it's something that would be relevant for logs and potentially other domains (than pure HTTP).

Proposal

I'm proposing to split the above two concerns by introducing an attribute-registry sub-dir under model. All attribute definitions will live in the attribute-registry:

All other sub-directories under /model represent usages of attributes. Thus, they would always only reference attributes that are defined in the registry. When attributes are being used in a certain domain (e.g. HTTP) the requirement level should be overwritten corresponding to the domain and signal. Same as it's already being done with references, other properties of the attribute (such as description, examples, etc.) can be overwritten for a specific domain / context as well.

That's how it would look like in the directory structure:

- ...
- model
   - attributes-registry             <-- ALL attributes MUST be defined here
      - http.yml
      - network.yml
      - faas.yml
      - ...
   - trace
      - http.yml
      - messaging.yaml
      - ...
   - metrics
      - http.yml
      - ...
   - resource
      - host.yml
      - cloud.yml
      - ...  

Benefits

If there's interest in realizing this or an adapted proposal, I'm happy to drive the realization.

jsuereth commented 1 year ago

We talked about this in the WG and we're very supportive of this approach going forward. I think there's a bit of planning / tooling fixes we'd want to do here, but please write up a more formal proposal!

AlexanderWert commented 1 year ago

I think there's a bit of planning / tooling fixes we'd want to do here, but please write up a more formal proposal!

Anything specific you have in mind?

I think tooling fixes would be required:

But those could happen as a second step.

I think the initial step could be just a refactoral change of the yaml files that should not affect the rendering and code generation of attributes (other than having links instead of native definition in the tables). How about I create a draft PR for HTTP attributes to have a concrete example how that could look like? And we take it from there?

joaopgrassi commented 1 year ago

One other shortcoming of the tools is this https://github.com/open-telemetry/build-tools/issues/183. Depending how the refactoring of attributes goes, we might bump into this issue.

dyladan commented 1 year ago

I am very supportive of this. I am actually in the process of rewriting https://github.com/open-telemetry/oteps/pull/224 and was considering including this in my proposal there. Having a separate definition of attributes will make it a lot easier to create experimental instrumentations which use them without the overhead of writing a full semconv for them.

AlexanderWert commented 1 year ago

I think the process for realizing this proposal could be the following:

  1. Do a refactoring of the yaml files by extracting all attribute definitions into corresponding /model/registry/<NAMESPACE>.yml files. This can happen namespace by namespace (similar to the refactoring we did with the markdown files). An example of how that would look like for HTTP is in this draft PR.
    • I'd propose to introduce a /docs/attributes-dictionary/ markdown directory that will plainly list all defined attributes per namespace.
    • This is a pure refactoring and must not change any semantics. Hence, only allowed changes in the corresponding markdown files will be
      • changed order of attributes in the tables (because of the way table generation orders attributes)
      • and links to the registry from the attribute usages.
    • As an intermediate step: allow rendering of attribute tables with omitted requirement_level column
  2. Adapt and fix tooling and yaml schema to enforce that structure
    • adapt the yaml syntax schema to explicitly differentiate between attribute definitions and semantic conventions that are using the attributes (e.g. not allowing to define prefix or attribute id in a semantic convention, or omitting the requirement_level for attribute definitions, etc. ). An initial proposal is here, to start discussing this direction:
    • adapt tooling to enforce that by throwing errors when attributes are defined in semantic conventions instead of referencing them
    • adapt tooling to render the dictionary with omitted requirement level column
    • adapt default requirement levels (opt_in for attribute definitions, recommended for attribute references)
  3. In a single PR, refactor the syntax in all YAML files to correspond to the new syntax and tooling requirements (from step 2) and update the tooling version to the one from step 2.

We can achieve the desired outcome already with step 1 (though the tooling won't technically prevent someone from breaking that structure again). Step 2 seems to require some more effort and IMHO can happen afterwards as it's rather a nice to have than a blocker for doing step 1.

So, I'd propose to start with step 1 by creating PR's similar to this draft PR.

@jsuereth @dyladan @joaopgrassi WDYT?

dyladan commented 1 year ago

@AlexanderWert I updated the OTEP and I believe we are in alignment on our goals. I'd recommend you give it a review and make sure there's nothing in there that conflicts with what you're trying to accomplish here. This PR would be one of the first steps in implementing what I proposed in the OTEP.

trisch-me commented 11 months ago

@AlexanderWert Should we close this issue as implemented? We are still have a lot of things to move to registry but as a concept it's already implemented and merged

AlexanderWert commented 11 months ago

@trisch-me We still need to do step 2 and 3 (i.e. making attribute definitions an explicit concept in the yaml schema + buildtools). So, I'd like to keep it open for now.

joaopgrassi commented 7 months ago

This was closed by mistake by the stale bot. Re-opening