Unnecessary call to config.Validate during component's initialization #33498
Comments
|
Pinging code owners:
See Adding Labels via Comments if you do not have permissions to add labels yourself. |
This was referenced Jun 19, 2024
This was referenced Jun 20, 2024
mx-psi
pushed a commit
that referenced
this issue
Jun 24, 2024
**Description:** This PR is upgrading dataset-go to version v0.19.0 - https://github.com/scalyr/dataset-go/releases/tag/v0.19.0 This PR is also fixing: * #33498 - config validation is not called during conversion, therefore I had to remove tests from logs and trace exporter. These tests are now moved to the factory. * #32533 - fixing failing integration tests, that were never updated and therefore the configuration contained invalid values **Link to tracking Issue:** #33498, #32533, #33675 **Testing:** Unit tests and integration tests. **Documentation:** <Describe the documentation added.> fixes #32533 --------- Signed-off-by: Alex Boten <[email protected]> Co-authored-by: Curtis Robert <[email protected]> Co-authored-by: Dmitrii Anoshin <[email protected]> Co-authored-by: Ishan Shanware <[email protected]> Co-authored-by: Evan Bradley <[email protected]> Co-authored-by: Tyler Helmuth <[email protected]> Co-authored-by: Alex Boten <[email protected]> Co-authored-by: Paulo Janotti <[email protected]> Co-authored-by: Joshua MacDonald <[email protected]> Co-authored-by: Rong Hu <[email protected]>
tomasmota
pushed a commit
to SpringerPE/opentelemetry-collector-contrib
that referenced
this issue
Jul 1, 2024
…try#33675) **Description:** This PR is upgrading dataset-go to version v0.19.0 - https://github.com/scalyr/dataset-go/releases/tag/v0.19.0 This PR is also fixing: * open-telemetry#33498 - config validation is not called during conversion, therefore I had to remove tests from logs and trace exporter. These tests are now moved to the factory. * open-telemetry#32533 - fixing failing integration tests, that were never updated and therefore the configuration contained invalid values **Link to tracking Issue:** open-telemetry#33498, open-telemetry#32533, open-telemetry#33675 **Testing:** Unit tests and integration tests. **Documentation:** <Describe the documentation added.> fixes open-telemetry#32533 --------- Signed-off-by: Alex Boten <[email protected]> Co-authored-by: Curtis Robert <[email protected]> Co-authored-by: Dmitrii Anoshin <[email protected]> Co-authored-by: Ishan Shanware <[email protected]> Co-authored-by: Evan Bradley <[email protected]> Co-authored-by: Tyler Helmuth <[email protected]> Co-authored-by: Alex Boten <[email protected]> Co-authored-by: Paulo Janotti <[email protected]> Co-authored-by: Joshua MacDonald <[email protected]> Co-authored-by: Rong Hu <[email protected]>
andrzej-stencel
added a commit
that referenced
this issue
Jul 11, 2024
**Description:** <Describe what has changed.> <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> Configuration validation is done during collector's startup, making it redundant when being called inside component's logic. This PR removes the Validate call done during Podman's receiver start function. **Link to tracking Issue:** <Issue number if applicable> #33498 Context: https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/32981/files#r1621849257 **Testing:** <Describe what testing was performed and which tests were added.> Tests moved to the `config_test.go` file following the testdata strategy. **Documentation:** <Describe the documentation added.> --------- Co-authored-by: Andrzej Stencel <[email protected]>
cparkins
pushed a commit
to AmadeusITGroup/opentelemetry-collector-contrib
that referenced
this issue
Jul 11, 2024
…try#33675) **Description:** This PR is upgrading dataset-go to version v0.19.0 - https://github.com/scalyr/dataset-go/releases/tag/v0.19.0 This PR is also fixing: * open-telemetry#33498 - config validation is not called during conversion, therefore I had to remove tests from logs and trace exporter. These tests are now moved to the factory. * open-telemetry#32533 - fixing failing integration tests, that were never updated and therefore the configuration contained invalid values **Link to tracking Issue:** open-telemetry#33498, open-telemetry#32533, open-telemetry#33675 **Testing:** Unit tests and integration tests. **Documentation:** <Describe the documentation added.> fixes open-telemetry#32533 --------- Signed-off-by: Alex Boten <[email protected]> Co-authored-by: Curtis Robert <[email protected]> Co-authored-by: Dmitrii Anoshin <[email protected]> Co-authored-by: Ishan Shanware <[email protected]> Co-authored-by: Evan Bradley <[email protected]> Co-authored-by: Tyler Helmuth <[email protected]> Co-authored-by: Alex Boten <[email protected]> Co-authored-by: Paulo Janotti <[email protected]> Co-authored-by: Joshua MacDonald <[email protected]> Co-authored-by: Rong Hu <[email protected]>
cparkins
pushed a commit
to AmadeusITGroup/opentelemetry-collector-contrib
that referenced
this issue
Jul 11, 2024
…y#33555) **Description:** <Describe what has changed.> <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> Configuration validation is done during collector's startup, making it redundant when being called inside component's logic. This PR removes the Validate call done during Podman's receiver start function. **Link to tracking Issue:** <Issue number if applicable> open-telemetry#33498 Context: https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/32981/files#r1621849257 **Testing:** <Describe what testing was performed and which tests were added.> Tests moved to the `config_test.go` file following the testdata strategy. **Documentation:** <Describe the documentation added.> --------- Co-authored-by: Andrzej Stencel <[email protected]>
This was referenced Jul 16, 2024
This was referenced Jul 30, 2024
abhishek-at-cloudwerx
pushed a commit
to Internal-Cloudwerx/opentelemetry-collector-contrib
that referenced
this issue
Aug 9, 2024
…y#33555) **Description:** <Describe what has changed.> <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> Configuration validation is done during collector's startup, making it redundant when being called inside component's logic. This PR removes the Validate call done during Podman's receiver start function. **Link to tracking Issue:** <Issue number if applicable> open-telemetry#33498 Context: https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/32981/files#r1621849257 **Testing:** <Describe what testing was performed and which tests were added.> Tests moved to the `config_test.go` file following the testdata strategy. **Documentation:** <Describe the documentation added.> --------- Co-authored-by: Andrzej Stencel <[email protected]>
|
This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping Pinging code owners:
See Adding Labels via Comments if you do not have permissions to add labels yourself. |
|
@rogercoll Do you know what components from your original list still do the unnecessary call? |
|
I think all but podman receiver. I will try to work on the docker one asap. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Component(s)
exporter/dataset, exporter/elasticsearch, exporter/opensearch, receiver/dockerstats, receiver/podman
Describe the issue you're reporting
All collector's components must implement the
component.ValidateConfiginterface. During collector's startup, it goes through all components and calls theValidatemethod. Its execution is finished if any component returns an error. Nonetheless, some components call the Validate method during its initialization, which is redundant taking into account the collector life cycle. (e.g. Podman receiver)I think that the
Validateinterface should be called by the configuration validator consumer (e.g. collector) instead of the configuration producers (components).Context for Podman's receiver: https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/32981/files#r1621849257
I linked to this issue the components that seems to be internally calling the configuration
Validatemethod. I would not say callingValidateis forbidden, just that might be redundant during collector's execution. However, feel free to unlink your component from this issue if the Validate method call is necessary for your specific use case or if it performs additional checks that are not covered during the startup validation phase.The text was updated successfully, but these errors were encountered: