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

Support Elastic Common Schema (ECS) in OpenTelemetry #222

Merged
merged 36 commits into from
Mar 18, 2023

Conversation

AlexanderWert
Copy link
Member

Reopening the OTEP (as the new owner of the OTEP) on supporting the Elastic Common Schema in OpenTelemetry (as a continuation of #199).

Would love to see the discussion from #199 to continue on this PR and approvals that have been already made on #199 being 'transitioned' / given here again.

AlexanderWert and others added 2 commits December 13, 2022 10:21
Co-authored-by: Georg Pirklbauer <georg.pirklbauer@dynatrace.com>
Copy link
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.

I'm supportive of continuing this work.

Copy link
Contributor

@tedsuo tedsuo left a comment

Choose a reason for hiding this comment

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

Excited this is finally happening! It's great to have ECS become part of the OpenTelemetry community.

@dyladan
Copy link
Member

dyladan commented Mar 17, 2023

@dyladan I suggest we use these fields as guides, and adopt them whenever OpenTelemetry expands into the relevant domain.

The reason I ask is because the security domain fields are specifically mentioned several times and some of them don't have obvious OTel applications.

Excited this is finally happening! It's great to have ECS become part of the OpenTelemetry community.

I agree this has been a long time coming and it's good to see some alignment.


Want to be clear that I'm not trying to be a thorn here, just want to make sure everyone is on the same page. I don't want to end up in the situation in the future where elastic folks feel like we reneged on the agreement if some of the fields without obvious applications take a long time to be integrated, but also don't want to end up in a situation where OTel is taking on the maintenance burden for fields which we're not really using.

@reyang
Copy link
Member

reyang commented Mar 17, 2023

I am blocking this since I don't support this OTEP in the current form. I still support the idea of converging the ECS and Otel semantic conventions but not without addressing this comment.
Furthermore it seems the existence of the OTEP is cited as a reason for proposing changes to existing conventions (for example), which is premature. It seems like the fact of an open OTEP with some approvals is signalling that it is guaranteed to be accepted and merged, which is not the case.
I am blocking the OTEP to signal it is not ready and should not be used a justification for specification changes yet.
I will remove my block when the open questions are address and we agree to move forward with the OTEP.

@tigrannajaryan I think this is resolved, could you take a look?

@tigrannajaryan I'll merge the OTEP now given we got a good set of approvals and your concerns have been addressed (I believe). Let's follow up offline once you are back from vacation and see if you have additional comments what we should follow up in another PR.

@reyang reyang merged commit 8b3e02c into open-telemetry:main Mar 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Completed Projects
Development

Successfully merging this pull request may close these issues.

None yet