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

Change ProbabilitySampler -> ParentBased in examples #3387

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

Conversation

KasonBraley
Copy link
Contributor

Update the examples to use the current API for configuring production sampling.

These names were changed a couple of years ago:
open-telemetry/opentelemetry-go#1115
open-telemetry/opentelemetry-go#1153

Update the examples to use the current API for configuring production sampling.
These names were changed a couple of years ago.

open-telemetry/opentelemetry-go#1115
open-telemetry/opentelemetry-go#1153
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Feb 16, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: KasonBraley / name: Kason Braley (222c2c2)

@KasonBraley KasonBraley changed the title Change ProbabilitySampler -> ParentBased Change ProbabilitySampler -> ParentBased in examples Feb 16, 2023
Copy link
Member

@dmathieu dmathieu left a comment

Choose a reason for hiding this comment

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

While this definitely makes sense, I wonder if we could make the doc more helpful with a link to https://opentelemetry.io/docs/instrumentation/go/exporting_data/#sampling?

@KasonBraley
Copy link
Contributor Author

While this definitely makes sense, I wonder if we could make the doc more helpful with a link to https://opentelemetry.io/docs/instrumentation/go/exporting_data/#sampling?

Good idea. Updated the comments to include the link - 8daebcc

@dmathieu dmathieu added the Skip Changelog Allow PR to succeed without requiring an addition to the CHANGELOG label Feb 22, 2023
@MadVikingGod MadVikingGod requested a review from a team February 23, 2023 16:01
@KasonBraley
Copy link
Contributor Author

KasonBraley commented Feb 24, 2023

open-telemetry/opentelemetry-go#3585 changed the location of the sampling documentation.

It changed from https://opentelemetry.io/docs/instrumentation/go/exporting_data/#sampling -> https://opentelemetry.io/docs/instrumentation/go/sampling/. I'll update the link in the comments - 18970fd.

@codecov
Copy link

codecov bot commented Feb 27, 2023

Codecov Report

Merging #3387 (18970fd) into main (d296842) will increase coverage by 16.6%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##            main   #3387      +/-   ##
========================================
+ Coverage   69.6%   86.2%   +16.6%     
========================================
  Files        147       4     -143     
  Lines       6919     124    -6795     
========================================
- Hits        4821     107    -4714     
+ Misses      1978      14    -1964     
+ Partials     120       3     -117     
Impacted Files Coverage Δ
...ntation/google.golang.org/grpc/otelgrpc/version.go
samplers/aws/xray/timer.go
...tation/github.com/labstack/echo/otelecho/config.go
samplers/aws/xray/rand.go
samplers/jaegerremote/version.go
...ation/github.com/gin-gonic/gin/otelgin/gintrace.go
samplers/jaegerremote/sampler_remote_options.go
samplers/aws/xray/internal/reservoir.go
instrumentation/net/http/otelhttp/transport.go
detectors/gcp/cloud-function.go
... and 138 more

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog Allow PR to succeed without requiring an addition to the CHANGELOG
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants