Skip to content
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

Fail when the builder can't load its configuration. #5731

Merged
merged 1 commit into from
Jul 25, 2022

Conversation

jpeach
Copy link
Contributor

@jpeach jpeach commented Jul 22, 2022

Description:

If the builder can't load its build configuration file, it logs an error
and continues. This results in building a collector that is not the one
the user specified. Exit with a fatal error instead.

Link to tracking Issue:

None.

Testing:

$ ./builder
2022-07-22T11:32:21.845+1000	INFO	internal/command.go:85	OpenTelemetry Collector distribution builder	{"version": "dev", "date": "unknown"}
2022-07-22T11:32:21.845+1000	FATAL	internal/command.go:93	failed to load config file	{"config-file": "$HOME/.otelcol-builder.yaml", "error": "open $HOME/.otelcol-builder.yaml: no such file or directory"}
go.opentelemetry.io/collector/cmd/builder/internal.initConfig
	/Users/jpeach/upstream/opentelemetry/opentelemetry-collector/cmd/builder/internal/command.go:93
go.opentelemetry.io/collector/cmd/builder/internal.Command.func1
	/Users/jpeach/upstream/opentelemetry/opentelemetry-collector/cmd/builder/internal/command.go:44
github.com/spf13/cobra.(*Command).execute
	/Users/jpeach/go/pkg/mod/github.com/spf13/[email protected]/command.go:872
github.com/spf13/cobra.(*Command).ExecuteC
	/Users/jpeach/go/pkg/mod/github.com/spf13/[email protected]/command.go:990
github.com/spf13/cobra.(*Command).Execute
	/Users/jpeach/go/pkg/mod/github.com/spf13/[email protected]/command.go:918
main.main
	/Users/jpeach/upstream/opentelemetry/opentelemetry-collector/cmd/builder/main.go:26
runtime.main
	/opt/homebrew/Cellar/go/1.18.4/libexec/src/runtime/proc.go:250
$ echo $?
1

@jpeach jpeach requested review from a team and mx-psi July 22, 2022 01:32
CHANGELOG.md Outdated Show resolved Hide resolved
cmd/builder/internal/command.go Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jul 22, 2022

Codecov Report

Merging #5731 (652a662) into main (3e14ee9) will decrease coverage by 0.03%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #5731      +/-   ##
==========================================
- Coverage   91.56%   91.53%   -0.04%     
==========================================
  Files         192      192              
  Lines       11411    11411              
==========================================
- Hits        10449    10445       -4     
- Misses        768      771       +3     
- Partials      194      195       +1     
Impacted Files Coverage Δ
pdata/internal/common.go 91.85% <0.00%> (-0.75%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3e14ee9...652a662. Read the comment docs.

If the builder can't load its build configuration file, it logs an error
and continues. This results in building a collector that is not the one
the user specified. Exit with a fatal error instead.

Signed-off-by: James Peach <[email protected]>
@@ -38,7 +38,9 @@ var (
// Command is the main entrypoint for this application
func Command() (*cobra.Command, error) {
cmd := &cobra.Command{
Use: "ocb",
SilenceUsage: true, // Don't print usage on Run error.
SilenceErrors: true, // Don't print errors; main does it.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not relevant to the PR: I remember that I've seen an issue with errors in the collector being printed twice, I feel that this was the issue.

@bogdandrutu bogdandrutu merged commit 9930104 into open-telemetry:main Jul 25, 2022
@jpeach jpeach deleted the fail-on-bad-config branch July 26, 2022 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants