[confighttp, configgrpc] Replace host parameter by map of extensions

[jade-guiton-dd]

Description

Alternative to #14162, where we make the map of extensions a mandatory positional parameter to replace host instead of an option.

Link to tracking issue

Fixes #13640

Testing

Existing tests make extensive use of the modified functions

Documentation

Added line in ToServer/ToClient(Conn) documentation describing the intended use of the extensions parameter.

[codecov]

Codecov Report

❌ Patch coverage is 76.66667% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.09%. Comparing base (974da01) to head (3a0c6c9).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
config/configgrpc/configgrpc.go	80.00%	1 Missing and 1 partial ⚠️
config/confighttp/client.go	71.42%	1 Missing and 1 partial ⚠️
config/confighttp/server.go	71.42%	1 Missing and 1 partial ⚠️
receiver/otlpreceiver/otlp.go	50.00%	0 Missing and 1 partial ⚠️

❌ Your patch status has failed because the patch coverage (76.66%) is below the target coverage (95.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #14190      +/-   ##
==========================================
- Coverage   92.15%   92.09%   -0.06%     
==========================================
  Files         667      667              
  Lines       41446    41453       +7     
==========================================
- Hits        38194    38178      -16     
- Misses       2215     2229      +14     
- Partials     1037     1046       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[jade-guiton-dd]

This new error message should hopefully make it clearer to users that, rather than their authentication/middleware extension mysteriously not being found despite being configured, this is likely because a component took a shortcut and passed nil instead of host.GetExtensions() for the extension map.

However, this relies on two assumptions:

1. Components will never intentionally pass in nil to disable the use of authentication/middlewares.
   Does that seem like a concern? Notably, @evan-bradley I think you mentioned intentionally disabling middlewares as a potential use case?
2. component.Host implementations will never return nil from GetExtensions, as opposed to an empty map. This happens to be true for the implementation provided by service, but isn't true for componenttest.NewNopHost(), and isn't documented in the documentation for the Host interface.
   Would it make sense to modify the nop host and documentation to match this new requirement? Could this be considered a breaking change in the stable component module?

[mx-psi]

Components will never intentionally pass in nil to disable the use of authentication/middlewares.
Does that seem like a concern?

That does not seem like a concern to me: if we want to add disabling middlewares as a use case later we can do it in a backwards compatible way if we start by returning an error now

Would it make sense to modify the nop host and documentation to match this new requirement? Could this be considered a breaking change in the stable component module?

I think that's fine, I feel like relying on that behavior seems like not something we should guarantee

[jade-guiton-dd]

That does not seem like a concern to me: if we want to add disabling middlewares as a use case later we can do it in a backwards compatible way if we start by returning an error now

To be clear, the only implication that "supporting disabling middlewares/authentication" would have is in the wording of the error (instead of saying "it's a bug in the calling component" we would explain that "middlewares and authentication are not available in this component")

[mx-psi]

That does not seem like a concern to me: if we want to add disabling middlewares as a use case later we can do it in a backwards compatible way if we start by returning an error now

To be clear, the only implication that "supporting disabling middlewares/authentication" would have is in the wording of the error (instead of saying "it's a bug in the calling component" we would explain that "middlewares and authentication are not available in this component")

Right, well that would also be backwards compatible (in general I would argue error -> is a backwards compatible change)

[evan-bradley]

I'm okay calling it a bug in the error message, as of right now there are no components that would want to intentionally disable middlewares. The only change I would suggest would be to encourage the user to report the bug, but I don't have any ideas right now that aren't overly wordy.

[jade-guiton-dd]

Based on the discussion in the maintainer meeting, I updated the component.Host doc wording to:

The returned map should only be nil if the host does not support extensions at all.

and the error message wording to:

authentication was / middlewares were configured but this component or its host does not support extensions

[codspeed-hq]

CodSpeed Performance Report

Merging #14190 will not alter performance

_{Comparing jade-guiton-dd:no-host-in-config-electric-boogaloo (3a0c6c9) with main (974da01)}

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

Summary

✅ 71 untouched