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

processor/otel: extend support for OpenTelemetry semantic conventions #4711

Merged
merged 5 commits into from
Feb 11, 2021

Conversation

axw
Copy link
Member

@axw axw commented Feb 10, 2021

Motivation/summary

Extend support for OpenTelemetry semantic conventions to cover what we support for traces in the OpenTelemetry Collector exporter, minus support for "exception" span events. These will be covered in a followup.

A couple of functional changes for the existing Jaeger support:

  • If the span type is not inferred, it will now be "app" rather than "custom". This aligns with the OpenTelemetry Collector exporter, and works better with the "time spent by span type" breakdown charts.
  • We no longer assume an outcome/result of "success" if the span didn't fail. We now default to "unknown", and only set "succcess" or "failure" if one of those outcomes is known, like we do in the OpenTelemetry Collector exporter and our own agents.

Checklist

How to test these changes

  1. Start apm-server
  2. Run an OpenTelemetry-instrumented application, configured to export OTLP to APM Server's standard host:port.
  3. Repeat step 2, exporting to an OpenTelemetry Collector with the elastic exporter.
  4. Compare the data received directly by APM Server to the data sent by the exporter.

Related issues

#4503

If the main "URL" input does not contain
a scheme, use the given scheme parameter.
@apmmachine
Copy link
Contributor

apmmachine commented Feb 10, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #4711 updated

  • Start Time: 2021-02-11T01:08:42.461+0000

  • Duration: 40 min 50 sec

  • Commit: a9d0473

Test stats 🧪

Test Results
Failed 0
Passed 4721
Skipped 124
Total 4845

Trends 🧪

Image of Build Times

Image of Tests

Steps errors 3

Expand to view the steps failures

Compress
  • Took 0 min 0 sec . View more details on here
  • Description: tar --exclude=coverage-files.tgz -czf coverage-files.tgz coverage
Compress
  • Took 0 min 0 sec . View more details on here
  • Description: tar --exclude=system-tests-linux-files.tgz -czf system-tests-linux-files.tgz system-tests
Test Sync
  • Took 4 min 48 sec . View more details on here
  • Description: ./.ci/scripts/sync.sh

Extend our support for OpenTelemetry semantic conventions.
@axw axw marked this pull request as ready for review February 10, 2021 10:47
@axw axw requested a review from a team February 10, 2021 10:48
Copy link
Contributor

@jalvz jalvz left a comment

Choose a reason for hiding this comment

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

Didn't run it, but LGTM. That's going to be a lot of fields for testing...

@axw axw merged commit d9f589a into elastic:master Feb 11, 2021
@axw axw deleted the otel-conventions branch February 11, 2021 01:54
axw added a commit to axw/apm-server that referenced this pull request Feb 19, 2021
axw added a commit to axw/apm-server that referenced this pull request Feb 19, 2021
axw added a commit that referenced this pull request Feb 19, 2021
axw added a commit that referenced this pull request Feb 19, 2021
@axw axw removed the test-plan label Feb 20, 2021
@simitt simitt self-assigned this Mar 2, 2021
@simitt
Copy link
Contributor

simitt commented Mar 4, 2021

Tested with BC 2 - works as expected.

Tested with the otel elastic exporter, otel otel/elastic exporter and directly exporting from a go app instrumented with the otel exporter. Stored information on transactions was the same.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants