Skip to content

Conversation

@joe-elliott
Copy link
Contributor

@joe-elliott joe-elliott commented Jan 16, 2020

Added propagation code/tests.

Fixes #492

cc @annanay25

@pjanotti
Copy link
Contributor

Hi @joe-elliott - is it the idea that this parameter is going to always be required?

@joe-elliott
Copy link
Contributor Author

@pjanotti

Yup. When Annanay added the remote sampling proxy functionality he forgot to add a way to configure the listening port. This PR amends that. Without this field remote sampling does not work.

@annanay25
Copy link
Contributor

Aha, I assumed that remote sampling would be served over the http proxy be default.

@joe-elliott
Copy link
Contributor Author

joe-elliott commented Jan 17, 2020

@pjanotti
I added a default host port. If none is specified it just uses Jaeger's default (5778). This way the operator only has to specify the fetch_endpoint and the it will automatically be hosted at default. This is now a valid config:

receivers:
  jaeger:
    protocols:
      grpc:
    remote_sampling:
      fetch_endpoint: "jaeger-collector:1234"

@codecov-io
Copy link

Codecov Report

Merging #507 into master will decrease coverage by <.01%.
The diff coverage is 71.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #507      +/-   ##
==========================================
- Coverage   76.04%   76.04%   -0.01%     
==========================================
  Files         123      123              
  Lines        7627     7634       +7     
==========================================
+ Hits         5800     5805       +5     
- Misses       1552     1553       +1     
- Partials      275      276       +1
Impacted Files Coverage Δ
receiver/jaegerreceiver/config.go 69.23% <ø> (ø) ⬆️
receiver/jaegerreceiver/factory.go 94.16% <71.42%> (-1.41%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dc11366...9fa86ab. Read the comment docs.

Copy link
Contributor

@pjanotti pjanotti left a comment

Choose a reason for hiding this comment

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

Thanks @joe-elliott! LGTM

@pjanotti pjanotti merged commit f72a421 into open-telemetry:master Jan 17, 2020
@joe-elliott
Copy link
Contributor Author

Thanks!

hughesjj pushed a commit to hughesjj/opentelemetry-collector that referenced this pull request Apr 27, 2023
* Release v0.29.0

* fixes

* Change the map type in expandStringValues (open-telemetry#507)

* fix smartagent receiver/extension config

* Update CHANGELOG.md

Co-authored-by: Jay Camp <[email protected]>
Co-authored-by: Paulo Janotti <[email protected]>
swiatekm pushed a commit to swiatekm/opentelemetry-collector that referenced this pull request Oct 9, 2024
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.

Provide a way to configure Jaeger Agent HTTP Port

5 participants