Created dry run flag for collector #6445

Chinwendu20 · 2022-10-30T20:12:24Z

Signed-off-by: Maureen [email protected]

Description: Created dry run flag for the collector

Link to tracking Issue: #4613

Documentation: Updated readme of service package but would create an issue on opentelemetry.io to update documentation

Signed-off-by: Maureen <[email protected]>

Chinwendu20 · 2022-11-08T23:48:28Z

Heyy, just discovered there were some recent changes to the main branch that would affect this PR. I would make some changes and push

bogdandrutu · 2022-11-10T19:30:35Z

service/config_provider.go

+	// DryRunGet validates the configuration, prints out all related errors without running the collector
+	DryRunGet(ctx context.Context, factories component.Factories) (*Config, error, bool)


Should "dry run" be a flag or a command?

/cc @codeboten @jpkrohling who also reviewed #6322

~~i would expect it to be a flag~~

Update: after looking at what helm does, i think it should just be a command. I would rename dry-run to verify or validate since that's what we're really asking the command to do here.

dry-run only really applies to the context of a command that would change a state somewhere.

jpkrohling · 2022-11-28T14:41:57Z

config/moved_config.go

+func (cfg *Config) DryRunValidate() {
+	// Currently, there is no default receiver enabled.
+	// The configuration must specify at least one receiver to be valid.
+	if len(cfg.Receivers) == 0 {


Each one of the conditions needs its test to ensure its correctness.

jpkrohling · 2022-11-28T14:45:57Z

service/internal/configunmarshaler/dry_run_unmarshaller.go

+
+func (ConfigUnmarshaler) DryRunUnmarshal(v *confmap.Conf, factories component.Factories) (*config.Config, error, bool) {
+	errorcheck := false
+	if err := v.Unmarshal(&rawCfg, confmap.WithErrorUnused()); err != nil {


Using a var that is defined in an unrelated file would be confusing. If you have to use the same var, it might make sense to split this into its own source file.

jpkrohling · 2022-11-28T14:47:24Z

service/internal/configunmarshaler/dry_run_unmarshaller.go

+	"go.opentelemetry.io/collector/confmap"
+)
+
+type ExtensionValidateError struct {


If those four types have the same definition, perhaps you can have only one? ComponentValidationError would probably be a good candidate here.

jpkrohling · 2022-11-28T14:47:54Z

service/internal/configunmarshaler/dry_run_unmarshaller.go

+	Err       string
+}
+
+var ExtensionValidateErrors []ExtensionValidateError


You can still have four vars, each being a slice of ComponentValidationErrors

Chinwendu20 · 2022-11-28T17:15:58Z

Thank you @jpkrohling. I have changed my approach to this problem because of recent changes to the main branch. I currently have it on my local computer and I am about wrapping up with tests. Would push as soon as it is done

Signed-off-by: Maureen <[email protected]>

…y-collector into add-dry-run-flag-validate-all-fields

Chinwendu20 · 2022-12-03T16:12:02Z

Still have to make some changes as recent changes in the main branch has affected my most recent push as well. I will try to finish this one up as fast as possible.

Signed-off-by: Maureen <[email protected]>

codecov · 2022-12-13T15:37:18Z

Codecov Report

Base: 90.15% // Head: 90.00% // Decreases project coverage by -0.14% ⚠️

Coverage data is based on head (ac2cbfe) compared to base (55e6fa0).
Patch coverage: 71.92% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6445      +/-   ##
==========================================
- Coverage   90.15%   90.00%   -0.15%     
==========================================
  Files         248      249       +1     
  Lines       14291    14403     +112     
==========================================
+ Hits        12884    12964      +80     
- Misses       1159     1183      +24     
- Partials      248      256       +8

Impacted Files	Coverage Δ
otelcol/command_validate.go	`44.00% <44.00%> (ø)`
otelcol/collector.go	`76.08% <62.50%> (-0.84%)`	⬇️
service/config.go	`90.68% <80.76%> (-9.32%)`	⬇️
otelcol/command.go	`66.66% <100.00%> (+1.14%)`	⬆️
otelcol/command_components.go	`89.28% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Chinwendu20 · 2022-12-13T20:40:58Z

Had a working solution earlier today just to find out that there are new changes to the the main branch later today hence this conflict.

Chinwendu20 · 2022-12-13T21:01:40Z

So the problem is that I am creating a new function for validate that just prints validation errors when it is encountered as opposed to returning quickly so if there is a change in the implementation of the main validate function I would have to change mine. I do not know if there is a smarter way of going about this

github-actions · 2022-12-28T03:15:56Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

jpkrohling · 2023-01-12T14:15:09Z

So the problem is that I am creating a new function for validate that just prints validation errors when it is encountered as opposed to returning quickly

It's okay -- I prefer to have all errors listed instead of failing fast in this case.

if there is a change in the implementation of the main validate function I would have to change mine

Not quite sure what you meant here.

Chinwendu20 · 2023-01-13T15:31:37Z

So the problem is that I am creating a new function for validate that just prints validation errors when it is encountered as opposed to returning quickly

It's okay -- I prefer to have all errors listed instead of failing fast in this case.

if there is a change in the implementation of the main validate function I would have to change mine

Not quite sure what you meant here.

I am giving a reason why I have had to keep changing my code and why some checks are failing. It is because at the time there were changes to the validate function after submitting my PR. I would go at it again

github-actions · 2023-01-28T03:16:04Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

github-actions · 2023-02-12T03:16:16Z

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@Chinwendu20

This is bringing back the functionality initially submitted in @Chinwendu20's PR: #6445 Adds a validate method which validates the configuration Link to tracking Issue: #4613 --------- Signed-off-by: Alex Boten <[email protected]> Co-authored-by: Maureen <[email protected]>

Created dry run for collector, fixes open-telemetry#4613

dd53f75

Signed-off-by: Maureen <[email protected]>

Chinwendu20 requested review from a team and codeboten October 30, 2022 20:12

Chinwendu20 added 3 commits October 30, 2022 22:01

Added chlog and updated readme

c022686

Signed-off-by: Maureen <[email protected]>

Added new service/testdata and added entry to chloggen

a8c81e3

Signed-off-by: Maureen <[email protected]>

Updated with main branch

7ddc5ee

Signed-off-by: Maureen <[email protected]>

bogdandrutu reviewed Nov 10, 2022

View reviewed changes

jpkrohling reviewed Nov 28, 2022

View reviewed changes

Chinwendu20 added 2 commits December 3, 2022 16:19

Updated code as a result of changes to the main branch

be8ffcc

Signed-off-by: Maureen <[email protected]>

Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…

7311780

…y-collector into add-dry-run-flag-validate-all-fields

Chinwendu20 added 3 commits December 13, 2022 10:16

Updated implementation in line with codebase changes

79d57a1

Signed-off-by: Maureen <[email protected]>

Merged branch with main and wrote test for newValidateSubCommand

e388d5e

Signed-off-by: Maureen <[email protected]>

Added test, linted, gofmted, fixed errors

ac2cbfe

Signed-off-by: Maureen <[email protected]>

github-actions bot added the Stale label Dec 28, 2022

bogdandrutu removed the Stale label Jan 4, 2023

github-actions bot added the Stale label Jan 28, 2023

github-actions bot closed this Feb 12, 2023

codeboten mentioned this pull request Jun 6, 2023

Add validate command to validate collector config #7835

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Created dry run flag for collector #6445

Created dry run flag for collector #6445

Chinwendu20 commented Oct 30, 2022 •

edited

Loading

Chinwendu20 commented Nov 8, 2022

bogdandrutu Nov 10, 2022

codeboten Nov 14, 2022 •

edited

Loading

jpkrohling Nov 28, 2022

jpkrohling Nov 28, 2022

jpkrohling Nov 28, 2022

jpkrohling Nov 28, 2022

Chinwendu20 commented Nov 28, 2022

Chinwendu20 commented Dec 3, 2022

codecov bot commented Dec 13, 2022

Chinwendu20 commented Dec 13, 2022

Chinwendu20 commented Dec 13, 2022

github-actions bot commented Dec 28, 2022

jpkrohling commented Jan 12, 2023

Chinwendu20 commented Jan 13, 2023 •

edited

Loading

github-actions bot commented Jan 28, 2023

github-actions bot commented Feb 12, 2023

		// DryRunGet validates the configuration, prints out all related errors without running the collector
		DryRunGet(ctx context.Context, factories component.Factories) (*Config, error, bool)

Created dry run flag for collector #6445

Created dry run flag for collector #6445

Conversation

Chinwendu20 commented Oct 30, 2022 • edited Loading

Chinwendu20 commented Nov 8, 2022

bogdandrutu Nov 10, 2022

Choose a reason for hiding this comment

codeboten Nov 14, 2022 • edited Loading

Choose a reason for hiding this comment

jpkrohling Nov 28, 2022

Choose a reason for hiding this comment

jpkrohling Nov 28, 2022

Choose a reason for hiding this comment

jpkrohling Nov 28, 2022

Choose a reason for hiding this comment

jpkrohling Nov 28, 2022

Choose a reason for hiding this comment

Chinwendu20 commented Nov 28, 2022

Chinwendu20 commented Dec 3, 2022

codecov bot commented Dec 13, 2022

Codecov Report

Chinwendu20 commented Dec 13, 2022

Chinwendu20 commented Dec 13, 2022

github-actions bot commented Dec 28, 2022

jpkrohling commented Jan 12, 2023

Chinwendu20 commented Jan 13, 2023 • edited Loading

github-actions bot commented Jan 28, 2023

github-actions bot commented Feb 12, 2023

Chinwendu20 commented Oct 30, 2022 •

edited

Loading

codeboten Nov 14, 2022 •

edited

Loading

Chinwendu20 commented Jan 13, 2023 •

edited

Loading