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

draft: Initial Event body fields support #236

Closed
wants to merge 1 commit into from
Closed

draft: Initial Event body fields support #236

wants to merge 1 commit into from

Conversation

MSNev
Copy link
Contributor

@MSNev MSNev commented Jul 10, 2024

  • Take over Event type to represent both Log and Span events
  • Add Body and Body Fields Support

github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 10, 2024
View reviewed changes
crates/weaver_semconv/src/body.rs Fixed Show fixed Hide fixed
crates/weaver_semconv/src/body.rs Fixed Show fixed Hide fixed
crates/weaver_semconv/src/body.rs Fixed Show fixed Hide fixed
github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 10, 2024
View reviewed changes
crates/weaver_resolver/src/attribute.rs Fixed Show fixed Hide fixed
crates/weaver_resolver/src/attribute.rs Fixed Show fixed Hide fixed
crates/weaver_resolver/src/attribute.rs Fixed Show fixed Hide fixed
crates/weaver_resolver/src/attribute.rs Fixed Show resolved Hide resolved
crates/weaver_resolver/src/attribute.rs Fixed Show fixed Hide fixed
crates/weaver_resolver/src/attribute.rs Fixed Show fixed Hide fixed
crates/weaver_resolver/src/attribute.rs Fixed Show fixed Hide fixed
crates/weaver_resolver/src/body.rs Fixed Show fixed Hide fixed
crates/weaver_resolver/src/registry.rs Fixed Show fixed Hide fixed
crates/weaver_resolver/src/registry.rs Fixed Show fixed Hide fixed
github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 11, 2024
View reviewed changes
crates/weaver_resolver/src/attribute.rs Fixed Show resolved Hide resolved
crates/weaver_resolver/src/attribute.rs Fixed Show resolved Hide resolved
lquerel
lquerel requested changes Jul 11, 2024
View reviewed changes
Copy link
Contributor

@lquerel lquerel left a comment

Choose a reason for hiding this comment

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

Great work on this PR! It's really cool to see. I have a few comments mainly about error handling and the use of the attribute catalog for body fields. Besides that, I think it's heading in the right direction. Thank you!

crates/weaver_resolved_schema/src/body.rs Outdated Show resolved Hide resolved
crates/weaver_resolver/src/attribute.rs Outdated Show resolved Hide resolved
crates/weaver_resolver/src/attribute.rs Outdated Show resolved Hide resolved
crates/weaver_resolver/src/attribute.rs Outdated Show resolved Hide resolved
crates/weaver_resolver/src/attribute.rs Fixed Show resolved Hide resolved
crates/weaver_resolver/src/body.rs Outdated Show resolved Hide resolved
crates/weaver_resolver/src/body.rs Outdated Show resolved Hide resolved
crates/weaver_resolver/src/body.rs Outdated Show resolved Hide resolved
crates/weaver_semconv/src/group.rs Outdated Show resolved Hide resolved
github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 16, 2024
View reviewed changes
crates/weaver_semconv/src/group.rs Fixed Show resolved Hide resolved
crates/weaver_semconv/src/group.rs Fixed Show resolved Hide resolved
crates/weaver_semconv/src/group.rs Fixed Show resolved Hide resolved
crates/weaver_semconv/src/group.rs Fixed Show resolved Hide resolved
crates/weaver_semconv/src/group.rs Fixed Show resolved Hide resolved
crates/weaver_semconv/src/group.rs Fixed Show resolved Hide resolved
@MSNev MSNev force-pushed the MSNev/EventSupport branch 2 times, most recently from 73c0654 to 4a3ed64 Compare July 16, 2024 01:02
github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 16, 2024
View reviewed changes
crates/weaver_semconv/src/group.rs Fixed Show resolved Hide resolved
crates/weaver_semconv/src/group.rs Fixed Show resolved Hide resolved
crates/weaver_semconv/src/group.rs Fixed Show resolved Hide resolved
@MSNev MSNev force-pushed the MSNev/EventSupport branch 3 times, most recently from 2d7bb5e to 6881311 Compare July 16, 2024 01:34
github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 16, 2024
View reviewed changes
crates/weaver_resolver/src/body.rs Fixed Show resolved Hide resolved
crates/weaver_resolver/src/body.rs Fixed Show resolved Hide resolved
crates/weaver_resolver/src/registry.rs Fixed Show resolved Hide resolved
crates/weaver_resolver/src/body.rs Fixed Show resolved Hide resolved
crates/weaver_resolver/src/body.rs Fixed Show resolved Hide resolved
crates/weaver_resolver/src/registry.rs Fixed Show resolved Hide resolved
@MSNev MSNev force-pushed the MSNev/EventSupport branch 2 times, most recently from 02babf8 to ff064c0 Compare July 16, 2024 01:44
@MSNev MSNev force-pushed the MSNev/EventSupport branch 3 times, most recently from b5b9234 to 1362718 Compare July 22, 2024 15:04
@MSNev
Initial Event body fields support
be0a32f 
- Take over Event type to represent both Log and Span events
- Add Body and Body Fields Support
- Fix and update forge tests
- Address initial review comments, add all existing events
- formatting fixes
- Address comments
- Sync changes
- Add semconv.jq queries
@MSNev MSNev changed the title draft: Initial Event body fields support Initial Event body fields support Aug 7, 2024
@MSNev MSNev marked this pull request as ready for review August 7, 2024 00:37
@MSNev MSNev marked this pull request as draft August 7, 2024 15:06
@MSNev MSNev changed the title Initial Event body fields support draft: Initial Event body fields support Aug 7, 2024
@MSNev MSNev mentioned this pull request Aug 7, 2024
Merged
@MSNev
Copy link
Contributor Author

MSNev commented Aug 7, 2024

#297 splits out the code implementation and basic registry checks from this PR, limiting the size and avoiding some of the additional "issues" as discussed in the tools Sig this morning.

Closing this PR, now that #297 is published

@MSNev MSNev closed this Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jsuereth jsuereth jsuereth left review comments

@lquerel lquerel lquerel requested changes

Assignees
No one assigned
Labels
None yet
Projects
Event API, SDK and Definition
Status: Done
OTel Weaver Project
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@MSNev @jsuereth @lquerel