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

Add comments pointing to the source of logging dimension names #4613

Closed
wants to merge 1 commit into from

Conversation

iliar-turdushev
Copy link
Contributor

@iliar-turdushev iliar-turdushev commented Oct 24, 2023

The PR adds links to OTel semantic conventions that are used for naming of logging dimensions.

Microsoft Reviewers: Open in CodeFlow

Comment on lines +12 to +17
// There is no OTel semantic convention for HTTP logs, therefore we are using
// semantic conventions for HTTP spans:
// https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-spans.md.
// Some dimensions (e.g. Host and Method) were not renamed, though they had corresponding
// names in the semantic conventions. The reason is that they are coming from ASP.NET Core 8
// HttpLogging component, therefore we cannot rename them.
Copy link
Member

Choose a reason for hiding this comment

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

Is this something we'd want end-users to see in the docs (e.g., at learn.microsoft), or is this for the maintainers only?
Ditto for the other comments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't mean to include this comment as public documentation, and my initial intent was to include it as a reference and justification "WHY" only for maintainers. It is part of public API though, and such a comment/remark could be useful for users.

Copy link
Contributor Author

@iliar-turdushev iliar-turdushev Oct 24, 2023

Choose a reason for hiding this comment

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

Regarding web doc, e.g. learn.microsoft, I think that we'll include into the doc a list of log dimensions and remark that some of those dimensions are following OTel semantic-conventions. As I understand, web doc will be developed separately, right?

@RussKie RussKie added waiting-author-feedback 📭 The author of this issue needs to respond in order for us to continue investigating this issue. and removed work in progress 🚧 area-telemetry labels Oct 24, 2023
@geeknoid
Copy link
Member

The PR mingles /// and // comments together, which isn't great. I think most of these added comments can go into /// as <remarks> sections.

@ghost ghost removed the waiting-author-feedback 📭 The author of this issue needs to respond in order for us to continue investigating this issue. label Oct 24, 2023
@iliar-turdushev
Copy link
Contributor Author

The PR mingles /// and // comments together, which isn't great. I think most of these added comments can go into /// as <remarks> sections.

@geeknoid

My initial intent was to include this kind of information only for maintainers, hence not as part of public API doc. We are currently following OTel conventions for spans, because there are no conventions for logs. It could be confusing to include links to span conventions into API documentation of Http and HttpClient logging.

We'll include information that the names of logging tags were inspired by OTel semantic conventions for spans into web documentation, e.g. learn.microsoft, and add there a justification why we are using span conventions.

I'm going to abandon the PR.

What do you think?

@geeknoid
Copy link
Member

Yeah, I think abandoning is reasonable.

@iliar-turdushev iliar-turdushev deleted the iliarturdu/otel-sem-links branch October 24, 2023 19:10
@ghost ghost locked as resolved and limited conversation to collaborators Nov 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants