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: improve open telemetry tracing instrumentation #2984

Merged
merged 11 commits into from
Apr 14, 2024

Conversation

thepabloaguilar
Copy link
Contributor

@thepabloaguilar thepabloaguilar commented Apr 14, 2024

This PR introduces two new configuration options under tracing section:

  • samplingRatio: Allow the users to define how much the sample they want to send to their collector, it's handy to let the user control the volume going out
  • propagators: Allow the users to define one or more propagators, this is specially useful when dealing with propagator migrations or in the environment where each team can decide what to use

Both additions are non breaking change as I kept the default value the same!

More information in the tracer resource was added as well as new span attributes for evaluation methods!

Additionally I've changed the imports of go.opentelemetry.io/otel/semconv to use the latest version as every import were using a different and old version! This also shouldn't break anything

Closes #2978

@thepabloaguilar thepabloaguilar requested a review from a team as a code owner April 14, 2024 04:24
Signed-off-by: Pablo Aguilar <pablo.aguilar@outlook.com.br>
Signed-off-by: Pablo Aguilar <pablo.aguilar@outlook.com.br>
Signed-off-by: Pablo Aguilar <pablo.aguilar@outlook.com.br>
Signed-off-by: Pablo Aguilar <pablo.aguilar@outlook.com.br>
Signed-off-by: Pablo Aguilar <pablo.aguilar@outlook.com.br>
Signed-off-by: Pablo Aguilar <pablo.aguilar@outlook.com.br>
Signed-off-by: Pablo Aguilar <pablo.aguilar@outlook.com.br>
Signed-off-by: Pablo Aguilar <pablo.aguilar@outlook.com.br>
Signed-off-by: Pablo Aguilar <pablo.aguilar@outlook.com.br>
Copy link

codecov bot commented Apr 14, 2024

Codecov Report

Attention: Patch coverage is 86.36364% with 9 lines in your changes are missing coverage. Please review.

Project coverage is 72.43%. Comparing base (f997fb9) to head (1b0f5ca).
Report is 232 commits behind head on main.

Files Patch % Lines
internal/cmd/grpc.go 60.00% 2 Missing and 2 partials ⚠️
internal/server/evaluator.go 0.00% 3 Missing ⚠️
internal/tracing/tracing.go 86.66% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2984      +/-   ##
==========================================
+ Coverage   70.78%   72.43%   +1.65%     
==========================================
  Files          91       95       +4     
  Lines        8729     7271    -1458     
==========================================
- Hits         6179     5267     -912     
+ Misses       2165     1607     -558     
- Partials      385      397      +12     

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

Signed-off-by: Pablo Aguilar <pablo.aguilar@outlook.com.br>
Copy link
Collaborator

@markphelps markphelps left a comment

Choose a reason for hiding this comment

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

looks great to me!! Thank you very much @thepabloaguilar ! Great work

@markphelps markphelps added needs docs Requires documentation updates automerge Used by Kodiak bot to automerge PRs labels Apr 14, 2024
@kodiakhq kodiakhq bot merged commit 3d5a345 into flipt-io:main Apr 14, 2024
26 checks passed
@thepabloaguilar thepabloaguilar deleted the issue-2978 branch April 15, 2024 13:09
@markphelps markphelps removed the needs docs Requires documentation updates label May 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Used by Kodiak bot to automerge PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve OpenTelemetry (OTLP) instrumentation
2 participants