Repository navigation
Conversation
|
CI failed due to libyaml, plz check |
d29e124 to
8ab01b8
Compare
There was a problem hiding this comment.
Change looks good to me, just some minor:
opentelemetry-exporter-otlp-proto-http 1.45.0 defaults to a urllib3 transport (via the new opentelemetry-exporter-http-transport) unless a requests.Session is passed. Salt doesn't pass one, so OTLP/HTTP no longer honours HTTP(S)_PROXY.
Could we either pass session=requests.Session() in salt/utils/metrics.py and salt/utils/tracing.py to keep the old behaviour, or document the change? A test that sets HTTP_PROXY and asserts the export goes through it would catch this.
Tests that use the real exporters
The existing tracing/metrics tests use in-memory or console exporters, so they'd still pass if an exporter broke. It would be good to add:
- A functional test that sends spans and metrics over OTLP/HTTP to a local receiver and decodes the request.
- Kick off a stress test to monitor prometheus works as expected?
8ab01b8 to
a833bc2
Compare
|
@twangboy needs rebase |
opentelemetry-exporter-otlp-proto-http 1.45.0 builds a urllib3 transport unless it is given a requests.Session, and that transport ignores the HTTP_PROXY and HTTPS_PROXY environment variables. Salt passed no session, so tracing and metrics exports stopped going through the configured proxy after the upgrade. Pass a requests.Session to the otlp-http span and metric exporters so the requests transport is used, as it was before 1.45.0. Add unit tests that the session is passed, and a functional test that sends spans and metrics over OTLP/HTTP to a local receiver, decodes the request, and checks the export is routed through HTTP_PROXY.
What does this PR do?
Update opentelemetry to 1.45.0
Merge requirements satisfied?
[NOTICE] Bug fixes or features added to Salt require tests.
Commits signed with GPG?
Yes