#229 Add OpenTelemetry Collector Server Auth Extensions to Receivers #2203
Conversation
|
|
clayton-cornell
added
the
type/docs
|
@grafana/grafana-alloy-maintainers I am going to hold back on reviewing the doc input until after a technical code review has been completed. There are some questions I have about the way things are phrased in the docs that may be resolved with the code review. |
Thanks for taking a look! Happy to clarify once the code review is complete. |
| "github.com/grafana/alloy/internal/component/otelcol" | ||
| otelcolCfg "github.com/grafana/alloy/internal/component/otelcol/config" | ||
| "github.com/grafana/alloy/internal/component/otelcol/receiver" | ||
| "github.com/grafana/alloy/internal/featuregate" |
There was a problem hiding this comment.
It looks like the previous committer used spaces instead of tabs and my code formatted did this. Let me know if you want me to revert this change.
| } | ||
|
|
||
| // Create an extension id based off the alloy name. For example | ||
| // auth.basic.creds will become auth.basic.creds.server/client depending on |
There was a problem hiding this comment.
nit: Does this have the slash in the name like the comment describes?
| notSupportedErr = ErrNotServerExtension | ||
| requiredAuthFeature = ServerAuthSupported | ||
| } | ||
| if t == Client { |
There was a problem hiding this comment.
Switch statement might be cleaner here? Or convert to else if? Right now it can only be client or server.
| "github.com/grafana/alloy/internal/component/otelcol" | ||
| otelcolCfg "github.com/grafana/alloy/internal/component/otelcol/config" | ||
| "github.com/grafana/alloy/internal/component/otelcol/receiver" | ||
| "github.com/grafana/alloy/internal/featuregate" |
|
Looks like some errors with the auth tests. |
Yeah, I dug into why this test was failing for a couple of hours and my assessment was that it was an issue with the test infrastructure incorrectly reporting that the component is running. See auth_test.go and I have a detailed comment explaining what is happening. Do you have any suggestions or comments on how to wait for the underlying extension to be running on the host? |
|
Will try to take a look at this but for full transparency most of the team is gonna be unavailable for a week or so due to the holidays. |
Not a problem, I'm not in any rush to get this PR merged. I did see a PR go through recently that added a /healthy endpoint to a component. I will try using that and let you know. Thanks for the help so far. |
|
The latest commits should fix the issue with the race condition in auth_basic.go. I have also addressed the outstanding comments on the PR. |
PR Description
Adds server authentication support to
otelcol.receivers.*that support the OpenTelemetry collector auth extension model. Some receivers are not wrappers of the collector so they are excluded from this PR.Receivers that now support server auth:
Which issue(s) this PR fixes
Fixes #229
Notes to the Reviewer
TestAuthServer in auth_basic_test.go is consistently failing due to a race condition in startup. Any ideas or potential solutions to this problem would be appreciated. The auth_basic plugin appears to function as expected when running from the binary.
User interface for client auth has not changed.
PR Checklist