-
Notifications
You must be signed in to change notification settings - Fork 488
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
flow: Add otelcol.receiver.vcenter component #5715
Conversation
docs/sources/flow/reference/components/otelcol.receiver.vcenter.md
Outdated
Show resolved
Hide resolved
docs/sources/flow/reference/components/otelcol.receiver.vcenter.md
Outdated
Show resolved
Hide resolved
docs/sources/flow/reference/components/otelcol.receiver.vcenter.md
Outdated
Show resolved
Hide resolved
docs/sources/flow/reference/components/otelcol.receiver.vcenter.md
Outdated
Show resolved
Hide resolved
require.NoError(t, ctrl.WaitRunning(time.Second)) | ||
} | ||
|
||
func TestArguments_UnmarshalRiver(t *testing.T) { |
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.
For unmarshal tests, I personally prefer the table driven approach. It allows you to add lots of tests with minimal overhead.
For example, I'd use the map[string]interface{}
style when some config structs are internal to OTel:
https://github.com/grafana/agent/blob/main/component/otelcol/processor/span/span_test.go
If none of the config structs are internal, I'd go for specifying the OTel structs directly:
https://github.com/grafana/agent/blob/main/component/otelcol/connector/servicegraph/servicegraph_test.go
docs/sources/flow/reference/components/otelcol.receiver.vcenter.md
Outdated
Show resolved
Hide resolved
docs/sources/flow/reference/components/otelcol.receiver.vcenter.md
Outdated
Show resolved
Hide resolved
docs/sources/flow/reference/components/otelcol.receiver.vcenter.md
Outdated
Show resolved
Hide resolved
…r.md Co-authored-by: Paulin Todev <[email protected]>
…r.md Co-authored-by: Paulin Todev <[email protected]>
…r.md Co-authored-by: Paulin Todev <[email protected]>
…r.md Co-authored-by: Paulin Todev <[email protected]>
Co-authored-by: Paulin Todev <[email protected]>
…r.md Co-authored-by: Paulin Todev <[email protected]>
docs/sources/flow/reference/components/otelcol.receiver.vcenter.md
Outdated
Show resolved
Hide resolved
docs/sources/flow/reference/components/otelcol.receiver.vcenter.md
Outdated
Show resolved
Hide resolved
docs/sources/flow/reference/components/otelcol.receiver.vcenter.md
Outdated
Show resolved
Hide resolved
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.
Thanks you, I'm happy for the PR to be merged. I do think we need to address the currently open comments on a follow-up PR though.
Also, we need more tests in the follow-up PR:
- tests for unmarshaling config
- a test which starts a mock http server and sends a mock payload, so that we can test that all metrics and attributes are streamed ok
type MetricsBuilderConfig struct { | ||
Metrics MetricsConfig `river:"metrics,block,optional"` | ||
ResourceAttributes ResourceAttributesConfig `river:"resource_attributes,block,optional"` | ||
} |
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.
In the follow-up PR we also need to document these options in our docs.
PR Description
This PR adds
otelcol.receiver.vcenter
which enables collection of metrics of vSheremachines.
PR Checklist