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.

Sign up for GitHub

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

Jump to bottom

Add semantic conventions for log record ID #3047

Merged
merged 38 commits into from
Apr 6, 2023
Merged

Add semantic conventions for log record ID #3047

merged 38 commits into from
Apr 6, 2023

Conversation

svrnm
Copy link
Member

@svrnm svrnm commented Dec 19, 2022

Fixes #597

Changes

Please provide a brief description of the changes here.

Some additional notes:

  • I kept the PR small, so I left out some other potential attributes, e.g. something for pre-existing ID (like windows event logs) or for storing the used logging/eventing system or even something like a "signature" that might be worth discussing, etc.
  • I followed the structure of "generic attributes" from the spans semconv
  • I took some of the existing wording from Decide if we need log record ID field #597 & Define Log Data model oteps#97 (comment) to describe the field

@svrnm svrnm requested review from a team December 19, 2022 17:29
@svrnm svrnm mentioned this pull request Dec 19, 2022
Closed
@arminru arminru added spec:logs Related to the specification/logs directory area:semantic-conventions Related to semantic conventions labels Dec 19, 2022
@djaglowski
Copy link
Member

Should we call this log.record.id, to match the pattern used by other log attributes?

@svrnm
Copy link
Member Author

svrnm commented Dec 27, 2022

Should we call this log.record.id, to match the pattern used by other log attributes?

works for me, I just picked log_record.id to start with something but I don't have a strong opinion for/against it.

@github-actions
Copy link

github-actions bot commented Jan 4, 2023

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Jan 4, 2023
@djaglowski djaglowski removed the Stale label Jan 4, 2023
@tigrannajaryan
Copy link
Member

Should we call this log.record.id, to match the pattern used by other log attributes?

I agree. We already have log. as a top-level namespace, let's use it. log.record_id is another alternate. If we think we need to add anything else about the record then log.record.id is a better choice.

joaopgrassi
joaopgrassi reviewed Jan 10, 2023
View reviewed changes
semantic_conventions/logs/log-general.yaml Outdated Show resolved Hide resolved
semantic_conventions/logs/log-general.yaml Outdated Show resolved Hide resolved
semantic_conventions/logs/log-general.yaml Outdated Show resolved Hide resolved
semantic_conventions/logs/log-general.yaml Outdated Show resolved Hide resolved
specification/logs/semantic_conventions/README.md Outdated Show resolved Hide resolved
jack-berg
jack-berg approved these changes Jan 12, 2023
View reviewed changes
specification/logs/semantic_conventions/general.md Outdated Show resolved Hide resolved
specification/logs/semantic_conventions/general.md Outdated Show resolved Hide resolved
@jack-berg jack-berg self-requested a review January 12, 2023 21:19
@jack-berg jack-berg mentioned this pull request Mar 2, 2023
Open
jack-berg
jack-berg approved these changes Mar 2, 2023
View reviewed changes
Copy link
Member

@jack-berg jack-berg left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I still think we should define how these can be automatically added by the SDK. Opened #3277 with a suggestion on how we might do so with another built-in LogRecordProcessor. Can add in a separate PR.

@svrnm
Copy link
Member Author

svrnm commented Mar 6, 2023

I still think we should define how these can be automatically added by the SDK. Opened #3277 with a suggestion on how we might do so with another built-in LogRecordProcessor. Can add in a separate PR.

Thank you.

joaopgrassi
joaopgrassi approved these changes Mar 6, 2023
View reviewed changes
semantic_conventions/logs/general.yaml Outdated Show resolved Hide resolved
@svrnm
Copy link
Member Author

svrnm commented Mar 13, 2023

Anything missing before this can be merged? TY

@github-actions
Copy link

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Mar 28, 2023
@svrnm
Copy link
Member Author

svrnm commented Mar 28, 2023

/unstale

@djaglowski djaglowski removed the Stale label Mar 28, 2023
@github-actions
Copy link

github-actions bot commented Apr 6, 2023

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Apr 6, 2023
@tigrannajaryan tigrannajaryan merged commit b124b1c into open-telemetry:main Apr 6, 2023
@tigrannajaryan
Copy link
Member

Thank you for the PR and for your patience @svrnm

@svrnm svrnm deleted the add-log-record-id-semconv branch January 11, 2024 14:38
carlosalberto pushed a commit to carlosalberto/opentelemetry-specification that referenced this pull request Oct 31, 2024
@svrnm @joaopgrassi
@jack-berg @tigrannajaryan
Add semantic conventions for log record ID (open-telemetry#3047)
beda134 
Fixes open-telemetry#597

## Changes

- Add a section for "generic attributes" to the log semconv
- Add an attribute `log_record.id` making use of ULID as discussed in
open-telemetry#597

Some additional notes:
- I kept the PR small, so I left out some other potential attributes,
e.g. something for pre-existing ID (like windows event logs) or for
storing the used logging/eventing system or even something like a
"signature" that might be worth discussing, etc.
- I followed the structure of "generic attributes" from the spans
semconv
- I took some of the existing wording from open-telemetry#597 &
open-telemetry/oteps#97 (comment) to
describe the field

---------

Signed-off-by: svrnm <[email protected]>
Co-authored-by: Joao Grassi <[email protected]>
Co-authored-by: jack-berg <[email protected]>
Co-authored-by: Tigran Najaryan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scheler scheler scheler left review comments

@jack-berg jack-berg jack-berg approved these changes

@reyang reyang reyang approved these changes

@joaopgrassi joaopgrassi joaopgrassi approved these changes

@djaglowski djaglowski djaglowski approved these changes

@tigrannajaryan tigrannajaryan tigrannajaryan approved these changes

Assignees
No one assigned
Labels
area:semantic-conventions Related to semantic conventions spec:logs Related to the specification/logs directory
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Decide if we need log record ID field
9 participants
@svrnm @djaglowski @tigrannajaryan @reyang @scheler @joaopgrassi @jack-berg @arminru @SergeyKanzhelev