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 <223565+codeboten@users.noreply.github.com> Co-authored-by: Curtis Robert <crobert@splunk.com> Co-authored-by: Dmitrii Anoshin <anoshindx@gmail.com> Co-authored-by: Ishan Shanware <shanware.ishan@gmail.com> Co-authored-by: Evan Bradley <11745660+evan-bradley@users.noreply.github.com> Co-authored-by: Tyler Helmuth <12352919+TylerHelmuth@users.noreply.github.com> Co-authored-by: Alex Boten <223565+codeboten@users.noreply.github.com> Co-authored-by: Paulo Janotti <pjanotti@splunk.com> Co-authored-by: Joshua MacDonald <jmacd@users.noreply.github.com> Co-authored-by: Rong Hu <hardproblems@users.noreply.github.com>
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 <andrzej.stencel@elastic.co>
This was referenced Jul 16, 2024
This was referenced Jul 30, 2024
|
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. |
mx-psi
pushed a commit
that referenced
this issue
Aug 28, 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 Dockers's receiver start function. **Link to tracking Issue:** <Issue number if applicable> #33498 **Testing:** <Describe what testing was performed and which tests were added.> No testing needed to be modified as the `Validate` functionality is already covered. **Documentation:** <Describe the documentation added.> --------- Co-authored-by: Sean Marciniak <30928402+MovieStoreGuy@users.noreply.github.com>
f7o
pushed a commit
to f7o/opentelemetry-collector-contrib
that referenced
this issue
Sep 12, 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 Dockers's receiver start function. **Link to tracking Issue:** <Issue number if applicable> open-telemetry#33498 **Testing:** <Describe what testing was performed and which tests were added.> No testing needed to be modified as the `Validate` functionality is already covered. **Documentation:** <Describe the documentation added.> --------- Co-authored-by: Sean Marciniak <30928402+MovieStoreGuy@users.noreply.github.com>
mx-psi
pushed a commit
that referenced
this issue
Sep 16, 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 exporter's constructor. **Link to tracking Issue:** <Issue number if applicable> #33498 **Testing:** <Describe what testing was performed and which tests were added.> Same coverage after removed lines: `coverage: 77.8% of statements` `TestFactory_CreateLogsExporter_Fail` and `TestFactory_CreateTracesExporter_Fail` functions are covered by the already available `config_test.go` cases. **Documentation:** <Describe the documentation added.>
|
jriguera
pushed a commit
to springernature/opentelemetry-collector-contrib
that referenced
this issue
Oct 4, 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 exporter's constructor. **Link to tracking Issue:** <Issue number if applicable> open-telemetry#33498 **Testing:** <Describe what testing was performed and which tests were added.> Same coverage after removed lines: `coverage: 77.8% of statements` `TestFactory_CreateLogsExporter_Fail` and `TestFactory_CreateTracesExporter_Fail` functions are covered by the already available `config_test.go` cases. **Documentation:** <Describe the documentation added.>
MovieStoreGuy
pushed a commit
that referenced
this issue
Oct 18, 2024
…35233) **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 exporter's constructor. **Link to tracking Issue:** <Issue number if applicable> #33498 (Last component, will close the issue) **Testing:** <Describe what testing was performed and which tests were added.> Added default config use case (validate error) **Documentation:** <Describe the documentation added.>
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: