Skip to content

Moved HTTP semantic conventions to a dedicated topic-based structure#141

Merged
reyang merged 6 commits intoopen-telemetry:mainfrom
AlexanderWert:md-restruct-http
Jun 28, 2023
Merged

Moved HTTP semantic conventions to a dedicated topic-based structure#141
reyang merged 6 commits intoopen-telemetry:mainfrom
AlexanderWert:md-restruct-http

Conversation

@AlexanderWert
Copy link
Copy Markdown
Member

@AlexanderWert AlexanderWert commented Jun 27, 2023

Part of #137

  • created /specification/README.md as a single entry point for all of the semantic conventions (for easier to consumption in the website)
  • created specification/http/README.md as an entry point for HTTP semantic conventions
  • moved HTTP conventions for spans and metrics to specification/http/
  • updated references and links

New structure for HTTP semantic conventions:

┬ specification
├─ README.md
└─ http
    ├─ README.md
    ├─ http-spans.md
    └─ http-metrics.md

@AlexanderWert
Moved HTTP semantic conventions to a dedicated topic-based structure
a4fd431 
Signed-off-by: Alexander Wert <alexander.wert@elastic.co>
@AlexanderWert AlexanderWert requested review from a team June 27, 2023 13:37
@AlexanderWert
minor clarification on intermediate navigation
da28699 
Signed-off-by: Alexander Wert <alexander.wert@elastic.co>
@AlexanderWert
fixed markdownlint issues
146d19e 
Signed-off-by: Alexander Wert <alexander.wert@elastic.co>
jsuereth
jsuereth approved these changes Jun 27, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@jsuereth jsuereth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM. However, looking at this, I'm curious if maybe we should go after "general" conventions first (or next). WDYT?

joaopgrassi
joaopgrassi reviewed Jun 27, 2023
View reviewed changes
@AlexanderWert
fixing comments from first reviews
2a134ea 
Signed-off-by: Alexander Wert <alexander.wert@elastic.co>
@joaopgrassi
Copy link
Copy Markdown
Member

joaopgrassi commented Jun 27, 2023 •
edited
Loading

.. I'm curious if maybe we should go after "general" conventions first (or next). WDYT?

Maybe bring "general" ones while bringing the ones that mostly use it? For ex, bring http-general as part of this, since we are moving http conventions which uses the bulk of general attributes?

@joaopgrassi
Copy link
Copy Markdown
Member

joaopgrassi commented Jun 27, 2023

It is happening! 🎉

@AlexanderWert
Copy link
Copy Markdown
Member Author

AlexanderWert commented Jun 27, 2023

.. I'm curious if maybe we should go after "general" conventions first (or next). WDYT?

Maybe bring "general" ones while bringing the ones that mostly use it? For ex, bring http-general as part of this, since we are moving http conventions which uses the bulk of general attributes?

@joaopgrassi What do you mean by http-general? I think, we do not have such a thing, yet. Or do you mean moving span-general.md as part of this PR as well because HTTP refers to many attributes from span-general.md?

@AlexanderWert
Copy link
Copy Markdown
Member Author

AlexanderWert commented Jun 27, 2023

I can do general next (tomorrow).

pyohannes
pyohannes approved these changes Jun 27, 2023
View reviewed changes
@joaopgrassi
Copy link
Copy Markdown
Member

joaopgrassi commented Jun 28, 2023

@AlexanderWert

What do you mean by http-general? I think, we do not have such a thing, yet. Or do you mean moving span-general.md as part of this PR as well because HTTP refers to many attributes from span-general.md?

Yes, sorry, I meant the span-general.md, not sure where I came from with http-general. All good to do it in a follow up :)

@AlexanderWert
Adapted description of the Semantic Conventions.
14bf685 
Signed-off-by: Alexander Wert <alexander.wert@elastic.co>
@AlexanderWert AlexanderWert mentioned this pull request Jun 28, 2023
Merged
@AlexanderWert
Copy link
Copy Markdown
Member Author

AlexanderWert commented Jun 28, 2023

@arminru Can we merge this? There are other PRs depending on this, so merging this would make it easier to review the other PRs.

@reyang
Copy link
Copy Markdown
Member

reyang commented Jun 28, 2023

@arminru Can we merge this? There are other PRs depending on this, so merging this would make it easier to review the other PRs.

+1 - let's move forward.

@reyang reyang merged commit cdaccc8 into open-telemetry:main Jun 28, 2023
This was referenced Jun 30, 2023
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jsuereth jsuereth jsuereth approved these changes

@joaopgrassi joaopgrassi joaopgrassi approved these changes

@pyohannes pyohannes pyohannes approved these changes

@reyang reyang reyang approved these changes

+1 more reviewer

@Oberon00 Oberon00 Oberon00 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@arminru arminru

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@AlexanderWert @joaopgrassi @reyang @jsuereth @Oberon00 @pyohannes @arminru