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

Jump to bottom

Allow setting the Sampler via environment variables #2517

Merged
merged 39 commits into from Mar 21, 2022
Merged

Allow setting the Sampler via environment variables #2517

merged 39 commits into from Mar 21, 2022

Conversation

vibhavp
Copy link
Contributor

@vibhavp vibhavp commented Jan 17, 2022 •
edited by pellared

Fixes #2305

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jan 17, 2022 •
edited

CLA Signed

The committers are authorized under a signed CLA.

@pellared
Copy link
Member

PTAL: https://github.com/open-telemetry/opentelemetry-go/blob/main/CONTRIBUTING.md#how-to-receive-comments

@vibhavp
Copy link
Contributor Author

vibhavp commented Jan 17, 2022

@pellared Thanks, I have signed the CLA.

@pellared pellared changed the title Allow setting the Sampler via environment variables (#2305) Allow setting the Sampler via environment variables Jan 17, 2022
@codecov
Copy link

codecov bot commented Jan 18, 2022 •
edited

Codecov Report

Merging #2517 (7c25f9f) into main (421d686) will increase coverage by 0.1%.
The diff coverage is 93.7%.

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #2517     +/-   ##
=======================================
+ Coverage   75.8%   75.9%   +0.1%     
=======================================
  Files        173     174      +1     
  Lines      11684   11748     +64     
=======================================
+ Hits        8864    8926     +62     
- Misses      2610    2612      +2     
  Partials     210     210
Impacted Files Coverage Δ
sdk/trace/sampler_env.go 91.4% <91.4%> (ø)
sdk/trace/provider.go 85.5% <100.0%> (+1.1%) ⬆️
sdk/trace/batch_span_processor.go 82.1% <0.0%> (+0.9%) ⬆️

jmacd
jmacd approved these changes Jan 18, 2022
View reviewed changes
sdk/trace/provider.go Outdated Show resolved Hide resolved
@andriikushch andriikushch mentioned this pull request Jan 19, 2022
Merged
pellared
pellared previously approved these changes Jan 19, 2022
View reviewed changes
sdk/trace/provider.go Show resolved Hide resolved
MadVikingGod
MadVikingGod reviewed Jan 19, 2022
View reviewed changes
Copy link
Contributor

@MadVikingGod MadVikingGod left a comment

Choose a reason for hiding this comment

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

Thank you for doing this work. It looks good.

sdk/trace/provider.go Outdated Show resolved Hide resolved
sdk/trace/provider.go Show resolved Hide resolved
sdk/trace/sampler_env.go Outdated Show resolved Hide resolved
sdk/trace/sampler_env.go Outdated Show resolved Hide resolved
sdk/trace/sampler_env.go Outdated Show resolved Hide resolved
sdk/trace/provider_test.go Outdated Show resolved Hide resolved
@pellared pellared dismissed their stale review January 19, 2022 17:06

I agree with MadVikingGod comments.

@vibhavp
Copy link
Contributor Author

vibhavp commented Jan 20, 2022

@MadVikingGod Thanks for the comments, I'll address them.

@MrAlias
Copy link
Contributor

MrAlias commented Feb 4, 2022

This appears to be stalled. @vibhavp do you still have time to address the issues identified in the comments?

@MrAlias MrAlias added this to the Release v1.4.0/v0.27.0 milestone Feb 4, 2022
@vibhavp
Copy link
Contributor Author

vibhavp commented Feb 4, 2022

Sorry for the delay! I've been a little busy, but I should be able to address the comments this weekend.

MrAlias
MrAlias reviewed Feb 9, 2022
View reviewed changes
sdk/trace/provider.go Outdated Show resolved Hide resolved
sdk/trace/sampler_env.go Show resolved Hide resolved
sdk/trace/provider_test.go Show resolved Hide resolved
@MrAlias
Copy link
Contributor

MrAlias commented Feb 9, 2022

Please be sure to fix the lint issue reported by the CI system.

@MrAlias MrAlias removed this from the Release v1.4.0/v0.27.0 milestone Feb 10, 2022
@MrAlias MrAlias added this to the Release v1.5.0 milestone Mar 2, 2022
MadVikingGod and others added 25 commits March 20, 2022 14:38
@MadVikingGod @vibhavp
Bump github.com/google/go-cmp from 0.5.6 to 0.5.7 across the project (#…
6a17fd9 
…2545)

* update go-cmp to 0.5.7

* fixes go.sums

Co-authored-by: Aaron Clawson <MadVikingGod@users.noreply.github.com>
@MadVikingGod @MrAlias @vibhavp
Un-escape url coding when parsing baggage. (#2529)
259bc50 
* un-escape url coding when parsing baggage.

* Added changelog

Co-authored-by: Aaron Clawson <MadVikingGod@users.noreply.github.com>
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
@MadVikingGod @MrAlias @vibhavp
Bump go.opentelemetry.io/proto/otlp from 0.11.0 to 0.12.0 (#2546)
cfe16a2 
* Update go.opentelemetry.io/proto/otlp to v0.12.0

* Changelog

* Update CHANGELOG.md

Fix's md linting

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>

Co-authored-by: Aaron Clawson <MadVikingGod@users.noreply.github.com>
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
@MrAlias @vibhavp
Remove unused sdk/internal/santize (#2549)
f064335
@MrAlias @vibhavp
Add links to code examples and docs (#2551)
1ea46ad
@dependabot @MrAlias @vibhavp
Bump github.com/prometheus/client_golang from 1.11.0 to 1.12.0 in /ex…
9c3a231 
…porters/prometheus (#2541)

* Bump github.com/prometheus/client_golang in /exporters/prometheus

Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.11.0 to 1.12.0.
- [Release notes](https://github.com/prometheus/client_golang/releases)
- [Changelog](https://github.com/prometheus/client_golang/blob/main/CHANGELOG.md)
- [Commits](prometheus/client_golang@v1.11.0...v1.12.0)

---
updated-dependencies:
- dependency-name: github.com/prometheus/client_golang
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* go mod tidy

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Tyler Yahn <codingalias@gmail.com>
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
@MrAlias @vibhavp
Optimize evictedQueue implementation and use (#2556)
fe8af71 
* Optimize evictedQueue impl and use

Avoid unnecessary allocations in the recordingSpan by using an
evictedQueue type instead of a pointer to one.

Lazy allocate the evictedQueue queue to prevent unnecessary operations
for spans without any use of the queue.

Document the evictedQueue

* Fix grammar
@sincejune @vibhavp
Add env support for batch span processor (#2515)
797d7df 
* Add env support for batch span processor

* Update changelog

* lint
@dependabot @MrAlias @vibhavp
Bump golang.org/x/tools from 0.1.8 to 0.1.9 in /internal/tools (#2566)
8b41a05 
* Bump golang.org/x/tools from 0.1.8 to 0.1.9 in /internal/tools

Bumps [golang.org/x/tools](https://github.com/golang/tools) from 0.1.8 to 0.1.9.
- [Release notes](https://github.com/golang/tools/releases)
- [Commits](golang/tools@v0.1.8...v0.1.9)

---
updated-dependencies:
- dependency-name: golang.org/x/tools
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

* Auto-fix go.sum changes in dependent modules

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: MrAlias <MrAlias@users.noreply.github.com>
@dependabot @MrAlias @vibhavp
Bump github.com/golangci/golangci-lint from 1.43.0 to 1.44.0 in /inte…
64a7b19 
…rnal/tools (#2567)

* Bump github.com/golangci/golangci-lint in /internal/tools

Bumps [github.com/golangci/golangci-lint](https://github.com/golangci/golangci-lint) from 1.43.0 to 1.44.0.
- [Release notes](https://github.com/golangci/golangci-lint/releases)
- [Changelog](https://github.com/golangci/golangci-lint/blob/master/CHANGELOG.md)
- [Commits](golangci/golangci-lint@v1.43.0...v1.44.0)

---
updated-dependencies:
- dependency-name: github.com/golangci/golangci-lint
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* Auto-fix go.sum changes in dependent modules

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: MrAlias <MrAlias@users.noreply.github.com>
@dependabot @MrAlias @vibhavp
Bump github.com/prometheus/client_golang from 1.12.0 to 1.12.1 in /ex…
48e7ea8 
…porters/prometheus (#2570)

* Bump github.com/prometheus/client_golang in /exporters/prometheus

Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.12.0 to 1.12.1.
- [Release notes](https://github.com/prometheus/client_golang/releases)
- [Changelog](https://github.com/prometheus/client_golang/blob/main/CHANGELOG.md)
- [Commits](prometheus/client_golang@v1.12.0...v1.12.1)

---
updated-dependencies:
- dependency-name: github.com/prometheus/client_golang
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

* Auto-fix go.sum changes in dependent modules

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: MrAlias <MrAlias@users.noreply.github.com>
@MrAlias @MadVikingGod @vibhavp
Fix TestBackoffRetry in otlp/internal/retry package (#2562)
a7e5d73 
* Fix TestBackoffRetry in otlp retry pkg

The delay of the retry is within two times a randomization factor (the
back-off time is delay * random number within [1 - factor, 1 + factor].
This means the waitFunc in TestBackoffRetry needs to check the delay is
within an appropriate delta, not equal to configure initial delay.

* Fix delta value

* Fix delta

Co-authored-by: Aaron Clawson <Aaron.Clawson@gmail.com>
@dependabot @MrAlias @vibhavp
Bump google.golang.org/grpc from 1.43.0 to 1.44.0 in /exporters/otlp/…
d5a0ed0 
…otlptrace (#2568)

* Bump google.golang.org/grpc in /exporters/otlp/otlptrace

Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.43.0 to 1.44.0.
- [Release notes](https://github.com/grpc/grpc-go/releases)
- [Commits](grpc/grpc-go@v1.43.0...v1.44.0)

---
updated-dependencies:
- dependency-name: google.golang.org/grpc
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* Auto-fix go.sum changes in dependent modules

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: MrAlias <MrAlias@users.noreply.github.com>
@dependabot @MrAlias @vibhavp
Bump google.golang.org/grpc from 1.43.0 to 1.44.0 in /example/otel-co…
45a1500 
…llector (#2565)

* Bump google.golang.org/grpc in /example/otel-collector

Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.43.0 to 1.44.0.
- [Release notes](https://github.com/grpc/grpc-go/releases)
- [Commits](grpc/grpc-go@v1.43.0...v1.44.0)

---
updated-dependencies:
- dependency-name: google.golang.org/grpc
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* Auto-fix go.sum changes in dependent modules

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: MrAlias <MrAlias@users.noreply.github.com>
@dependabot @MrAlias @vibhavp
Bump google.golang.org/grpc from 1.43.0 to 1.44.0 in /exporters/otlp/…
8dc7c69 
…otlpmetric (#2572)

* Bump google.golang.org/grpc in /exporters/otlp/otlpmetric

Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.43.0 to 1.44.0.
- [Release notes](https://github.com/grpc/grpc-go/releases)
- [Commits](grpc/grpc-go@v1.43.0...v1.44.0)

---
updated-dependencies:
- dependency-name: google.golang.org/grpc
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

* Auto-fix go.sum changes in dependent modules

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: MrAlias <MrAlias@users.noreply.github.com>
@MrAlias @vibhavp
Change Options to accept type not pointer (#2558)
3fe7c09 
* Change trace options to accept type not pointer

Add benchmark to show allocation improvement.

* Update CONTRIBUTING.md guidelines

* Update all Option iface

* Fix grammar in CONTRIBUTING
@MrAlias @vibhavp
Do not store TracerProvider or Tracer fields in SDK recordingSpan (#2575
409cd54 
)

* Do not store TracerProvider fields in span

Instead of keeping a reference to the span's Tracer, and therefore also
it's TracerProvider, and the associated resource and spanLimits just
keep the reference to the Tracer. Refer to the TracerProvider fields
when needed instead.

* Make span refer to the inst lib via the Tracer

Instead of holding a field in the span, refer to the field in the parent
Tracer.
@chalin @vibhavp
[website_docs] fix page meta-links (#2580)
f31ea0a 
Contributes to open-telemetry/opentelemetry.io#1096

/cc @cartermp @austinlparker
@dmathieu @MadVikingGod @vibhavp
Validate members once, in NewMember (#2522)
0be1658 
* use NewMember, or specify if the member is not validated when creating new ones

* expect members to already be validated when creating a new package

* add changelog entry

* add an isEmpty field to member and property for quick validation

* rename isEmpty to hasData

So by default, an empty struct really is marked as having no data

* Update baggage/baggage_test.go

Co-authored-by: Aaron Clawson <Aaron.Clawson@gmail.com>

* don't validate the member in parseMember, we alredy ran that validation

We also don't want to use NewMember, as that runs the property
validation again, making the benchmark quite slower

* move changelog entry to the fixed section

* provide the member/property data when returning an invalid error

Co-authored-by: Aaron Clawson <Aaron.Clawson@gmail.com>
@MrAlias @vibhavp
Fix link to Zipkin exporter (#2581)
0354823 
Currently it is linked to the old package that was moved.
@MrAlias @vibhavp
Unexport EnvBatchSpanProcessor* constants (#2583)
edc91fd 
* Move BSP env support to internal

* Use pkg name

* Update env test

* Use internal/env in sdk/trace
@vibhavp
Avoid an extra allocation in applyTracerProviderEnvConfigs.
d3a3e0c
@vibhavp
Add additional errors for ratio > 1.0.
e512343
@vibhavp
Add test cases for ratio > 1.0.
3af1c81
@vibhavp
Merge branch 'main' into sdk-env-var-opts
10fafb8
MrAlias
MrAlias approved these changes Mar 21, 2022
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
@MrAlias MrAlias merged commit fdbcf71 into open-telemetry:main Mar 21, 2022
@MrAlias MrAlias mentioned this pull request Mar 23, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jmacd jmacd jmacd approved these changes

@MrAlias MrAlias MrAlias approved these changes

@Aneurysm9 Aneurysm9 Awaiting requested review from Aneurysm9 Aneurysm9 is a code owner

@dashpole dashpole Awaiting requested review from dashpole dashpole is a code owner

@evantorrie evantorrie Awaiting requested review from evantorrie evantorrie is a code owner

@paivagustavo paivagustavo Awaiting requested review from paivagustavo

@XSAM XSAM Awaiting requested review from XSAM XSAM is a code owner

@MadVikingGod MadVikingGod Awaiting requested review from MadVikingGod MadVikingGod is a code owner

@pellared pellared Awaiting requested review from pellared pellared is a code owner

@hanyuancheung hanyuancheung Awaiting requested review from hanyuancheung hanyuancheung is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Release v1.6.0
Development

Successfully merging this pull request may close these issues.

Support the OTEL_TRACES_SAMPLER* environment variables