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

Add schema URL support to Tracer #1889

Merged

Conversation

tigrannajaryan
Copy link
Member

@tigrannajaryan tigrannajaryan commented May 7, 2021

This adds support for schema URL to the Tracer according to the specification:
open-telemetry/opentelemetry-specification#1666
(Link to replaced by the link to the spec after that PR is merged)

For the future: once the proto is updated we will need to populate the
schema_url field in the messages.

@tigrannajaryan
Copy link
Member Author

@Aneurysm9 @MrAlias I am looking for your feedback to understand if this is the right direction.

@codecov
Copy link

codecov bot commented May 7, 2021

Codecov Report

Merging #1889 (faa7c69) into main (c1f460e) will increase coverage by 0.0%.
The diff coverage is 87.5%.

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #1889   +/-   ##
=====================================
  Coverage   79.6%   79.7%           
=====================================
  Files        141     141           
  Lines       7486    7492    +6     
=====================================
+ Hits        5964    5973    +9     
+ Misses      1273    1271    -2     
+ Partials     249     248    -1     
Impacted Files Coverage Δ
trace/config.go 75.5% <75.0%> (-0.1%) ⬇️
exporters/otlp/internal/transform/span.go 98.1% <100.0%> (+<0.1%) ⬆️
sdk/trace/provider.go 84.5% <100.0%> (+0.1%) ⬆️
exporters/trace/jaeger/jaeger.go 94.5% <0.0%> (+1.0%) ⬆️
sdk/trace/batch_span_processor.go 87.1% <0.0%> (+1.5%) ⬆️

@tigrannajaryan tigrannajaryan force-pushed the feature/tigran/add-schema-url branch from 25bd614 to 52c430c Compare May 7, 2021 15:48
sdk/trace/provider.go Outdated Show resolved Hide resolved
Copy link
Contributor

@MrAlias MrAlias left a comment

Choose a reason for hiding this comment

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

This looks good to me. Suggestions are nit-picks.

CHANGELOG.md Outdated Show resolved Hide resolved
trace/config.go Outdated Show resolved Hide resolved
This adds support for schema URL to the Tracer according to the specification:
open-telemetry/opentelemetry-specification#1666
(Link to replaced by the link to the spec after that PR is merged)

For the future: once the proto is updated we will need to populate the
schema_url field in the messages.
@tigrannajaryan tigrannajaryan changed the title [WIP] Add schema URL support to Tracer Add schema URL support to Tracer May 27, 2021
}

// InstrumentationVersion returns the version of the library providing instrumentation.
func (t *TracerConfig) InstrumentationVersion() string {
return t.instrumentationVersion
}

// SchemaURL returns the Schema URL of the telemetry emitted by the Tracer.
func (t *TracerConfig) SchemaURL() string {
return t.schemaURL
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 covered by TestSchemaURL, not sure why codecov says it is not.

@tigrannajaryan
Copy link
Member Author

@MrAlias @Aneurysm9 this is ready for review.

@Aneurysm9 Aneurysm9 merged commit bd93586 into open-telemetry:main May 27, 2021
@Aneurysm9 Aneurysm9 mentioned this pull request Jun 17, 2021
tigrannajaryan added a commit to open-telemetry/opentelemetry-specification that referenced this pull request Jul 21, 2021
SergeyKanzhelev pushed a commit to open-telemetry/opentelemetry-specification that referenced this pull request Jul 21, 2021
@tigrannajaryan tigrannajaryan deleted the feature/tigran/add-schema-url branch May 2, 2022 16:16
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.

None yet

3 participants