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

feat: sem_conv stability mode #1728

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

hannahramadan
Copy link

@hannahramadan hannahramadan commented Sep 12, 2024

Lays the foundation for accepting a new environment variable, OTEL_SEMCONV_STABILITY_OPT_IN, which gives users the ability to determine which semantic conventions to use. Individual instrumentation are expected to add metrics/attributes by calling the methods present in OpenTelemetry::SemanticConventions::StabilityMode. This borrows from Pythons implementation.

Only HTTP is supported for now (spec) and the values defined so far are:

  • http - emit the new, stable HTTP and networking conventions only
  • http/dup - emit both the old and the stable HTTP and networking conventions
  • default - continue emitting whatever version of the old experimental HTTP and networking conventions the instrumentation was emitting previously.
  • Note: http/dup has higher precedence than http in case both values are present

@hannahramadan hannahramadan changed the title feat: introduce sem_comv stability mode feat: introduce sem_conv stability mode Sep 13, 2024
@hannahramadan hannahramadan changed the title feat: introduce sem_conv stability mode feat: sem_conv stability mode Sep 17, 2024
@hannahramadan hannahramadan marked this pull request as ready for review September 17, 2024 16:05
@kaylareopelle
Copy link
Contributor

Link to instrumentation that uses this PR: open-telemetry/opentelemetry-ruby-contrib#1166

@kaylareopelle kaylareopelle added the spec-compliance Required for OpenTelemetry spec compliance label Oct 2, 2024
HTTP_DUP = 'http/dup'

# These constants will be pulled in from elsehwere once semvconv stability PR merged
# https://github.com/open-telemetry/opentelemetry-ruby/pull/1651
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like there's a few stable attributes that aren't included in this PR. Why did you choose to use these attributes and not others?

https://opentelemetry.io/docs/specs/semconv/http/http-spans/#http-client

@lock.synchronize do
opt_in = ENV.fetch('OTEL_SEMCONV_STABILITY_OPT_IN', nil)
opt_in_list = opt_in.split(',').map(&:strip) if opt_in
http_set_sability_mode(opt_in_list) if opt_in_list
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
http_set_sability_mode(opt_in_list) if opt_in_list
http_set_stability_mode(opt_in_list) if opt_in_list

end
end

def http_set_sability_mode(opt_in_list)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
def http_set_sability_mode(opt_in_list)
def http_set_stability_mode(opt_in_list)

Copy link
Contributor

github-actions bot commented Nov 2, 2024

👋 This pull request has been marked as stale because it has been open with no activity. You can: comment on the issue or remove the stale label to hold stale off for a while, add the keep label to hold stale off permanently, or do nothing. If you do nothing this pull request will be closed eventually by the stale bot

@github-actions github-actions bot added the stale label Nov 2, 2024
@kaylareopelle kaylareopelle added keep and removed stale labels Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keep spec-compliance Required for OpenTelemetry spec compliance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants