-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[exporter/signalfxexporter] Add all HTTP client settings to the configuration #16837
[exporter/signalfxexporter] Add all HTTP client settings to the configuration #16837
Conversation
Foresight Summary
View More Details⭕ build-and-test-windows workflow has finished in 5 seconds (44 minutes 50 seconds less than
|
Job | Failed Steps | Tests | |
---|---|---|---|
windows-unittest-matrix | - 🔗 | N/A | See Details |
windows-unittest | - 🔗 | N/A | See Details |
✅ tracegen workflow has finished in 1 minute (3 minutes 1 second less than main
branch avg.) and finished at 26th Dec, 2022.
Job | Failed Steps | Tests | |
---|---|---|---|
build-dev | - 🔗 | N/A | See Details |
publish-latest | - 🔗 | N/A | See Details |
publish-stable | - 🔗 | N/A | See Details |
✅ check-links workflow has finished in 1 minute 30 seconds (2 minutes 9 seconds less than main
branch avg.) and finished at 26th Dec, 2022.
Job | Failed Steps | Tests | |
---|---|---|---|
changed files | - 🔗 | N/A | See Details |
check-links | - 🔗 | N/A | See Details |
✅ changelog workflow has finished in 1 minute 39 seconds (6 minutes 24 seconds less than main
branch avg.) and finished at 26th Dec, 2022.
Job | Failed Steps | Tests | |
---|---|---|---|
changelog | - 🔗 | N/A | See Details |
✅ prometheus-compliance-tests workflow has finished in 3 minutes 17 seconds (6 minutes 45 seconds less than main
branch avg.) and finished at 26th Dec, 2022.
Job | Failed Steps | Tests | |
---|---|---|---|
prometheus-compliance-tests | - 🔗 | ✅ 21 ❌ 0 ⏭ 0 🔗 | See Details |
✅ load-tests workflow has finished in 8 minutes 6 seconds (9 minutes 16 seconds less than main
branch avg.) and finished at 26th Dec, 2022.
Job | Failed Steps | Tests | |
---|---|---|---|
loadtest (TestIdleMode) | - 🔗 | ✅ 1 ❌ 0 ⏭ 0 🔗 | See Details |
loadtest (TestTraceAttributesProcessor) | - 🔗 | ✅ 3 ❌ 0 ⏭ 0 🔗 | See Details |
loadtest (TestTraceNoBackend10kSPS|TestTrace1kSPSWithAttrs) | - 🔗 | ✅ 8 ❌ 0 ⏭ 0 🔗 | See Details |
loadtest (TestMetric10kDPS|TestMetricsFromFile) | - 🔗 | ✅ 6 ❌ 0 ⏭ 0 🔗 | See Details |
loadtest (TestTraceBallast1kSPSWithAttrs|TestTraceBallast1kSPSAddAttrs) | - 🔗 | ✅ 10 ❌ 0 ⏭ 0 🔗 | See Details |
loadtest (TestMetricResourceProcessor|TestTrace10kSPS) | - 🔗 | ✅ 12 ❌ 0 ⏭ 0 🔗 | See Details |
loadtest (TestBallastMemory|TestLog10kDPS) | - 🔗 | ✅ 19 ❌ 0 ⏭ 0 🔗 | See Details |
setup-environment | - 🔗 | N/A | See Details |
✅ build-and-test workflow has finished in 34 minutes 42 seconds (24 minutes 25 seconds less than main
branch avg.) and finished at 26th Dec, 2022.
Job | Failed Steps | Tests | |
---|---|---|---|
integration-tests | - 🔗 | ✅ 55 ❌ 0 ⏭ 0 🔗 | See Details |
cross-compile (darwin, amd64) | - 🔗 | N/A | See Details |
cross-compile (darwin, arm64) | - 🔗 | N/A | See Details |
cross-compile (linux, 386) | - 🔗 | N/A | See Details |
cross-compile (linux, arm) | - 🔗 | N/A | See Details |
cross-compile (linux, arm64) | - 🔗 | N/A | See Details |
cross-compile (linux, amd64) | - 🔗 | N/A | See Details |
cross-compile (linux, ppc64le) | - 🔗 | N/A | See Details |
cross-compile (windows, 386) | - 🔗 | N/A | See Details |
cross-compile (windows, amd64) | - 🔗 | N/A | See Details |
build-package (deb) | - 🔗 | N/A | See Details |
build-package (rpm) | - 🔗 | N/A | See Details |
windows-msi | - 🔗 | N/A | See Details |
publish-check | - 🔗 | N/A | See Details |
publish-dev | - 🔗 | N/A | See Details |
publish-stable | - 🔗 | N/A | See Details |
*You can configure Foresight comments in your organization settings page.
2f956ee
to
a3471d2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple of comments but I do appreciate making the change so quickly
02f9b4f
to
5e4cc13
Compare
I am fine with these changes, however, @dmitryax and @pmcollins should verify these. |
e50fda8
to
5ef3ff9
Compare
…lFx exporter configuration
Co-authored-by: Dmitrii Anoshin <anoshindx@gmail.com>
Co-authored-by: Dmitrii Anoshin <anoshindx@gmail.com>
…them out before setting them to nil
5ed0867
to
cef13bf
Compare
Ready to merge once @dmitryax or @pmcollins approves. |
se.config.HTTPClientSettings.TLSSetting = se.config.IngestTLSSettings | ||
|
||
if se.config.HTTPClientSettings.MaxIdleConns == nil { | ||
se.config.HTTPClientSettings.MaxIdleConns = &se.config.MaxConnections |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to add a deprecation warning log about it. Otherwise, the user will not notice the deprecation. Can be done in a follow-up PR
…g parameter (open-telemetry#27611) **Description:** The max_connections config setting were deprecated with open-telemetry#16837 and open-telemetry#16838. It has been 10 months since these changes have been made and it is time to remove this configuration setting altogether. **Link to tracking Issue:** open-telemetry#27610
…g parameter (open-telemetry#27611) **Description:** The max_connections config setting were deprecated with open-telemetry#16837 and open-telemetry#16838. It has been 10 months since these changes have been made and it is time to remove this configuration setting altogether. **Link to tracking Issue:** open-telemetry#27610
Description:
Expose all HTTP client settings to the exporter configuration so they may be set in addition to the existing flags.
Link to tracking Issue:
#16807
Testing:
Unit tests all pass.
Documentation:
N/A - not sure we need to shore up those config settings explicitly as I haven't seen that done elsewhere - please advise.