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

Support Environment Variable for Jaeger Remote Sampler #6310

Merged
merged 11 commits into from
Nov 15, 2024

Conversation

akstron
Copy link
Contributor

@akstron akstron commented Nov 8, 2024

Which problem is this PR solving?

Resolves #6256

Description of the changes

  • Added capability to configure jaeger remote sampler using OTEL_TRACES_SAMPLER_ARG
  • Environment variables would be applied before the code values for the corresponding keys.
  • If the corresponding values exist in code, we take those.

@akstron akstron changed the title Support Environment Variable for Jaeger Remote Sampler [WIP] Support Environment Variable for Jaeger Remote Sampler Nov 8, 2024
@akstron akstron changed the title [WIP] Support Environment Variable for Jaeger Remote Sampler Support Environment Variable for Jaeger Remote Sampler Nov 9, 2024
@akstron akstron marked this pull request as ready for review November 9, 2024 06:54
@akstron akstron requested a review from a team as a code owner November 9, 2024 06:54
samplers/jaegerremote/sampler_remote_options.go Outdated Show resolved Hide resolved
samplers/jaegerremote/sampler_remote_options.go Outdated Show resolved Hide resolved
samplers/jaegerremote/sampler_remote_options.go Outdated Show resolved Hide resolved
samplers/jaegerremote/sampler_remote_test.go Outdated Show resolved Hide resolved
samplers/jaegerremote/sampler_remote_test.go Show resolved Hide resolved
@akstron akstron force-pushed the jaeger-remote-sampler branch from 7b9893e to 063e396 Compare November 10, 2024 06:23
Copy link

codecov bot commented Nov 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.8%. Comparing base (717e1db) to head (d489629).

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #6310   +/-   ##
=====================================
  Coverage   66.7%   66.8%           
=====================================
  Files        193     193           
  Lines      15565   15605   +40     
=====================================
+ Hits       10391   10431   +40     
  Misses      4883    4883           
  Partials     291     291           
Files with missing lines Coverage Δ
samplers/jaegerremote/sampler_remote_options.go 100.0% <100.0%> (ø)

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.

Could you add a changelog entry?

@akstron
Copy link
Contributor Author

akstron commented Nov 12, 2024

Hi @dmathieu I have added the changelog entry. Thanks

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Signed-off-by: Alok Kumar Singh <[email protected]>
Signed-off-by: Alok Kumar Singh <[email protected]>
Signed-off-by: Alok Kumar Singh <[email protected]>
Signed-off-by: Alok Kumar Singh <[email protected]>
@akstron akstron force-pushed the jaeger-remote-sampler branch from 82f32e8 to 463b00d Compare November 12, 2024 11:25
Signed-off-by: Alok Kumar Singh <[email protected]>
CHANGELOG.md Show resolved Hide resolved
Co-authored-by: Damien Mathieu <[email protected]>
@akstron
Copy link
Contributor Author

akstron commented Nov 14, 2024

Hi @dmathieu , can we get this merged? Thanks

@dmathieu dmathieu merged commit c18691f into open-telemetry:main Nov 15, 2024
22 checks passed
@MrAlias MrAlias added this to the v1.33.0 milestone Dec 11, 2024
MrAlias added a commit that referenced this pull request Dec 12, 2024
### Added

- Added support for providing `endpoint`, `pollingIntervalMs` and
`initialSamplingRate` using environment variable
`OTEL_TRACES_SAMPLER_ARG` in
`go.opentelemetry.io/contrib/samples/jaegerremote`. (#6310)
- Added support exporting logs via OTLP over gRPC in
`go.opentelemetry.io/contrib/config`. (#6340)
- The `go.opentelemetry.io/contrib/bridges/otellogr` module.
This module provides an OpenTelemetry logging bridge for
`github.com/go-logr/logr`. (#6386)
- Added SNS instrumentation in
`go.opentelemetry.io/contrib/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws`.
(#6388)

### Changed

- Change the span name to be `GET /path` so it complies with the OTel
HTTP semantic conventions in
`go.opentelemetry.io/contrib/instrumentation/github.com/labstack/echo/otelecho`.
(#6365)
- Record errors instead of setting the `gin.errors` attribute in
`go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin`.
(#6346)
- The `go.opentelemetry.io/contrib/config` now supports multiple schemas
in subdirectories (i.e. `go.opentelemetry.io/contrib/config/v0.3.0`) for
easier migration. (#6412)

### Fixed

- Fix broken AWS presigned URLs when using instrumentation in
`go.opentelemetry.io/contrib/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws`.
(#5975)
- Fixed the value for configuring the OTLP exporter to use `grpc`
instead of `grpc/protobuf` in `go.opentelemetry.io/contrib/config`.
(#6338)
- Allow marshaling types in `go.opentelemetry.io/contrib/config`.
(#6347)
- Removed the redundant handling of panic from the `HTML` function in
`go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin`.
(#6373)
- The `code.function` attribute emitted by
`go.opentelemetry.io/contrib/bridges/otelslog` now stores just the
function name instead the package path-qualified function name. The
`code.namespace` attribute now stores the package path. (#6415)
- The `code.function` attribute emitted by
`go.opentelemetry.io/contrib/bridges/otelzap` now stores just the
function name instead the package path-qualified function name. The
`code.namespace` attribute now stores the package path. (#6423)
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.

Support Environment Variable Installation and Configuration of Jaeger Remote Sampler
4 participants