-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[confighttp] add confighttp.NewDefaultServerConfig() #10275
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #10275 +/- ##
=======================================
Coverage 92.27% 92.28%
=======================================
Files 397 397
Lines 18723 18736 +13
=======================================
+ Hits 17277 17290 +13
Misses 1086 1086
Partials 360 360 ☔ View full report in Codecov by Sentry. |
2267d43
to
c06259c
Compare
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
b7852d4
to
fe60e85
Compare
…alize structs as part of the default server config
I discussed this in person with @atoulme yesterday, and I missed the fact that we don't currently expose the timeouts to our end-users. I'm OK with this PR as is, but would prefer to have that option exposed first, and a default being set here, so that we have fewer breaking changes. |
fe60e85
to
d9f4c60
Compare
If we add the timeout fields, they take over/conflict with the fields declared for one receiver in contrib. I think we could skip the test in contrib, merge this, fix in contrib. WDYT? |
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
What's the status of this one? |
It's ready to merge, if we are ok with breaking the build with contrib, or we can merge open-telemetry/opentelemetry-collector-contrib#34181 to get a passing build. |
See open-telemetry/opentelemetry-collector#10275 for context - we need to disable a test so we can merge the change in core, and bring it here next.
I merged the contrib PR. |
@jpkrohling thanks the tests now pass. |
Description
add a new default method to instantiate the HTTP server config.
Link to tracking issue
#9655