-
Notifications
You must be signed in to change notification settings - Fork 179
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
Feature Request: Restructure Semantic Convention markdown to be topical vs. by signal #137
Comments
I started working on this. I came up with the following proposal for the structure:
Will also create a (draft) PR with the proposed structure. |
There has been a similar attempt in the past, see this PR: open-telemetry/opentelemetry-specification#1977 If I remember correctly, one big open question was how to also keep the YAML structure in line with the markdown changes (open-telemetry/opentelemetry-specification#2019). |
In the semconv meeting from yesterday (26.06.2023), we discussed that the idea would be to keep the YAML as they are, and just modify the rendering part for easy of consumption by readers. The YAML as they are are important for code generation apart from generating the markdown. I guess we could if we want later on to make the YAML match the structure of the markdown, but I don't think it's required. @jsuereth did I got it right? |
Thoughts on whether we should do the change as one big PR vs. moving things around in multiple smaller PRs? |
Here's another issue, which seems to overlaps with this one: open-telemetry/opentelemetry-specification#2397 It contains this paragraph:
Is it a goal for this feature request to tackle this issue of duplication? Currently shared attributes need to be duplicated in YAML files, and consequently they are duplicated in the resulting markdown. |
I think some of the yaml files are already created to be signal-agnostic (i.e. the ones in the root directory here: https://github.com/open-telemetry/semantic-conventions/tree/main/semantic_conventions). Those attributes are then referenced in other yaml files (rather than duplicating them). IMHO we need to make this the default (i.e. defining attributes as being signal-agnostic and only referencing them from the specific contexts / signal definitions). Still, I think it can be done as a separate step after restructuring the markdown (so my understanding is, it should not be part of this issue). |
The tooling supports specifying a (primary) type for conventions independent of directory structure, and @lmolkova recently added improvements for filtering for that in code generators open-telemetry/build-tools#157 |
Maybe moving "one signal at a time" is better? No idea if that really helps much |
I was rather thinking of doing it by topic (e.g. starting with With a bigger change I fear it would be difficult to resolve conflicts with other things going on (as it would imply that files are being moved around, deleted, created) |
Yes I'd like to do the migration separately here. Our primary goal right now is the following:
I'd actually suggest move one category at a time. From the proposal here, I think we should do the following:
|
Working on that. Will create a PR for that first step today. |
I love the proposal, but some feedback:
My counter-proposal:
|
Regarding "technology-general/cloud-provider-general", why are they under "general"? Shouldn't these have their own folders? (only logs-general/metrics-general/spans-general make sense to me)
|
Right now we have one example, that's somehow unclear where to put:
It's sort of clear where to put DynamoDB, S3 attributes, etc. But where to put the general AWS SDK attributes? So that was the idea behind: |
Shove them under "aws"?
I feel these are "general" for AWS clients and services, I view "general" as:
|
PRs for restructuring / moving all of the semantic conventions are created:
All of the PRs depend on #143. Once all of the above PRs are merged, we'll need a final "clean-up" PR that would remove "old" |
Closing since the restructuring is done. Some new issues were discovered and tracked separately, e.g. #157. |
Today, the semantic conventions are split by signal (Resource, Trace, Logs, Metrics). Instead we should have topical semantic conventions, e.g. Http, Database, etc.
This bug tracks the idea and work associated with refactoring.
The text was updated successfully, but these errors were encountered: