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

Convert OTel semantic convention to Datadog's #3273

Merged
merged 3 commits into from
Nov 22, 2023
Merged

Conversation

marcotc
Copy link
Member

@marcotc marcotc commented Nov 22, 2023

What does this PR do?

This PR parses the relevant OpenTelemetry semantic conventions for tracing and converts those values into appropriate Datadog APM values.

Most changes come from parsing OTel Span attributes to derive a Datadog Span operation name.
For example, from Semantic Conventions for Database Client Calls:
If an OTel span has the attribute db.system and the Span kind :client,
then the respective Datadog Span will have the operation name "#{otel_span.attributes['db.system']}.query".

This PR also introduces a few reserved attributes that can be set on an OTel Span to modify a Datadog Span field, for example resource.name can be set to modify the Datadog Span's resource name.

There are also a few fixes to attribute parsing, for cross-language consistency.

Motivation:

OpenTelemetry consistent, both with the OpenTelemetry semantic conventions and with other Datadog APM languages.

How to test the change?

There are unit test and system-tests for this change: DataDog/system-tests#1456

For Datadog employees:

  • If this PR touches code that signs or publishes builds or packages, or handles
    credentials of any kind, I've requested a review from @DataDog/security-design-and-guidance.
  • This PR doesn't touch any of that.

Unsure? Have a question? Request a review!

@marcotc marcotc requested review from a team as code owners November 22, 2023 00:10
@codecov-commenter
Copy link

codecov-commenter commented Nov 22, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (3becb40) 98.22% compared to head (2c06b2d) 98.22%.

Files Patch % Lines
lib/datadog/opentelemetry/sdk/span_processor.rb 93.33% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           master    #3273    +/-   ##
========================================
  Coverage   98.22%   98.22%            
========================================
  Files        1253     1253            
  Lines       72290    72393   +103     
  Branches     3361     3393    +32     
========================================
+ Hits        71009    71111   +102     
- Misses       1281     1282     +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@marcotc marcotc self-assigned this Nov 22, 2023
@github-actions github-actions bot removed the tracing label Nov 22, 2023
# semantic convention for span kind and span attributes.
# @see https://opentelemetry.io/docs/specs/semconv/general/trace/

# rubocop:disable Metrics/CyclomaticComplexity,Metrics/PerceivedComplexity
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps we could revisit this later to break this method into smaller pieces.

kwargs[:service] = attributes['service.name'] if attributes.key?('service.name')
kwargs[:type] = attributes['span.type'] if attributes.key?('span.type')

attributes.reject! { |key, _| OpenTelemetry::Trace::Span::DATADOG_SPAN_ATTRIBUTE_OVERRIDES.include?(key) }

# DEV: There's no `flat_map!`; we have to split it into two operations
Copy link
Contributor

Choose a reason for hiding this comment

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

For later: Would it be possible to complete this with a single operation with reduce or each_with_object?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is an interesting line because I built it to support arbitrarily nested arrays, but that's actually not required for OTel, so I might simplify it later. But I wasn't able to in time for this PR.

@marcotc marcotc merged commit a58330f into master Nov 22, 2023
218 checks passed
@marcotc marcotc deleted the swap-op-with-resource branch November 22, 2023 18:40
@github-actions github-actions bot added this to the 1.17.0 milestone Nov 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants