Add "components" Command #6322

Chinwendu20 · 2022-10-15T13:53:48Z

Signed-off-by: Maureen amaka013@gmail.com

Description:
Enhancement: Added build command to output components in collector distribution in YAML format

Link to tracking Issue: Closes #4671

Documentation: I would open an issue on opentelemetry.io for the documentation, however, I updated the readme of the service package.

@jpkrohling please check this out

Chinwendu20 · 2022-10-15T17:36:04Z

Missed adding the distribution version, adding it now

Chinwendu20 · 2022-10-15T17:54:57Z

Hi, please I noticed something about the flag, it is always, true. I am fixing that now

Update: Fixed it

Signed-off-by: Maureen <amaka013@gmail.com>

codeboten

Thanks for the contribution @Chinwendu20! Please add a unit test for the new functionality.

Adding entries to the changelog is done via yaml files in the .chloggen directory. Please take a look at https://github.com/open-telemetry/opentelemetry-collector/blob/main/CONTRIBUTING.md#adding-a-changelog-entry for all the details!

service/flags.go

codecov · 2022-10-17T20:26:25Z

Codecov Report

Base: 91.66% // Head: 91.13% // Decreases project coverage by -0.52% ⚠️

Coverage data is based on head (b2bba0b) compared to base (f411094).
Patch coverage: 89.65% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6322      +/-   ##
==========================================
- Coverage   91.66%   91.13%   -0.53%     
==========================================
  Files         240      245       +5     
  Lines       13815    14143     +328     
==========================================
+ Hits        12663    12889     +226     
- Misses        921     1002      +81     
- Partials      231      252      +21

Impacted Files	Coverage Δ
service/command_build_info.go	`89.28% <89.28%> (ø)`
service/command.go	`65.51% <100.00%> (+1.23%)`	⬆️
consumer/consumertest/base_consumer.go	`0.00% <0.00%> (-100.00%)`	⬇️
component/component.go	`63.82% <0.00%> (-36.18%)`	⬇️
component/componenttest/nop_factories.go	`0.00% <0.00%> (-29.42%)`	⬇️
processor/batchprocessor/metrics.go	`84.71% <0.00%> (-15.29%)`	⬇️
receiver/otlpreceiver/internal/logs/otlp.go	`87.50% <0.00%> (-12.50%)`	⬇️
receiver/otlpreceiver/internal/metrics/otlp.go	`87.50% <0.00%> (-12.50%)`	⬇️
receiver/otlpreceiver/internal/trace/otlp.go	`88.00% <0.00%> (-12.00%)`	⬇️
exporter/exporterhelper/obsreport.go	`94.91% <0.00%> (-5.09%)`	⬇️
... and 45 more

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.

Signed-off-by: Maureen <amaka013@gmail.com>

Chinwendu20 · 2022-10-18T07:29:51Z

I have pushed the new change for the changelog and unit test. I am still waiting for your response to the "logger.info" recommendation, thank you.

jpkrohling

I tried it locally and this is what I got:

$ ./bin/otelcorecol_linux_amd64 --build-info
version: 0.62.1-dev
receivers:
    - otlp
processors:
    - batch
    - memory_limiter
exporters:
    - otlphttp
    - logging
    - otlp
extensions:
    - memory_ballast
    - zpages

This looks nice!

jpkrohling · 2022-10-19T00:25:23Z

service/flags.go

 )

 var (
 	// Command-line flag that control the configuration file.
 	gatesList = featuregate.FlagValue{}
+	BuildFlag bool


This should not be public

Okay, thanks. I would be changing that.

jpkrohling · 2022-10-19T00:25:45Z

service/flags.go

-	"strings"
-
+	"fmt"
+	"go.opentelemetry.io/collector/config"


I wonder why the linter didn't catch this. The import order seems odd.

I ran gofmt -w . on the project. This is how it looks:

Should I look up correct import order and implement changes manually if this is not in line?

Run make lint and see if it complains. I expect it to complain with something like this:

service/flags.go:20: File is not `goimports`-ed with -local go.opentelemetry.io/collector (goimports) "fmt"

You can then run the following command to fix it automatically for you:

make fmt

jpkrohling · 2022-10-19T00:26:04Z

service/flags.go

 )

 type configFlagValue struct {
 	values []string
 	sets   []string
 }

+type componentsOutput struct {
+	Version string
+


Can you remove the extraneous empty lines?

Ok, changing that. Thanks

service/flags.go

jpkrohling · 2022-10-19T00:27:00Z

service/flags.go

+	components.Version = set.BuildInfo.Version
+
+	yamlData, err := yaml.Marshal(components)
+


Remove this empty line, keeping the error check close to where the error is received.

Ok, thank you. On it.

service/flags.go

jpkrohling · 2022-10-19T00:28:14Z

service/flags_test.go

+	}
+	ExpectedYamlStruct := componentsOutput{
+		Version: "latest",
+


Reconsider the empty lines here as well.

Ok thank you

service/command.go

Chinwendu20 · 2022-10-19T05:04:18Z

I tried it locally and this is what I got:

$ ./bin/otelcorecol_linux_amd64 --build-info
version: 0.62.1-dev
receivers:
    - otlp
processors:
    - batch
    - memory_limiter
exporters:
    - otlphttp
    - logging
    - otlp
extensions:
    - memory_ballast
    - zpages

This looks nice!

Thank youu

Co-authored-by: Juraci Paixão Kröhling <juraci.github@kroehling.de>

jpkrohling · 2022-10-19T11:56:26Z

Once this is ready for review again, please ping me. Note that there's a merge conflict now that needs to be solved.

Chinwendu20 · 2022-10-19T19:55:30Z

Okay, thank you. On it.

Signed-off-by: Maureen <amaka013@gmail.com>

Chinwendu20 · 2022-10-20T04:46:20Z

Thanks @jpkrohling. I have pushed with the changes.

Chinwendu20 · 2022-10-20T08:14:39Z

Oh missed writing a test. I would push that soon

Signed-off-by: Maureen <amaka013@gmail.com>

Chinwendu20 · 2022-10-20T11:25:38Z

I have pushed with the test @jpkrohling

Signed-off-by: Maureen <amaka013@gmail.com>

jpkrohling

This looks good to me, but @Aneurysm9 and @codeboten had comments as well, I would want to request their approval in addition to mine.

jpkrohling · 2022-10-20T17:44:07Z

service/command.go


 	"github.com/spf13/cobra"
+	"gopkg.in/yaml.v3"
+
+	"go.opentelemetry.io/collector/config"



Those two imports should be together (go.opentelemetry.io/collector/...) but I think the linter will tell you that as well :-)

Thanks I attempted putting them together but when I ran "make fmt", it sorted them back in this order

Chinwendu20 · 2022-11-08T23:24:16Z

Thank you all for your time and patience with me. I have fixed the lint test but the build test, am afraid would still fail. I ran "make test" locally and it fails probably because there is no test in the telemetry package. I would need your guidance:

Similar to what is happening on the CI: https://github.com/open-telemetry/opentelemetry-collector/actions/runs/3414103074/jobs/5682633344

Signed-off-by: Maureen <amaka013@gmail.com>

codeboten

You can see the issue here:

=== RUN   TestNewBuildSubCommand
==================
WARNING: DATA RACE
Write at 0x00c000209b50 by goroutine 17:
  go.opentelemetry.io/collector/service.TestNewBuildSubCommand.func1()
      /home/runner/work/opentelemetry-collector/opentelemetry-collector/service/command_build_info_test.go:73 +0xdd

Previous write at 0x00c000209b50 by goroutine 42:
  go.opentelemetry.io/collector/service.TestNewBuildSubCommand()
      /home/runner/work/opentelemetry-collector/opentelemetry-collector/service/command_build_info_test.go:78 +0xce4
  testing.tRunner()
      /opt/hostedtoolcache/go/1.18.7/x64/src/testing/testing.go:1439 +0x213
  testing.(*T).Run.func1()
      /opt/hostedtoolcache/go/1.18.7/x64/src/testing/testing.go:1486 +0x47

Goroutine 17 (running) created at:
  go.opentelemetry.io/collector/service.TestNewBuildSubCommand()
      /home/runner/work/opentelemetry-collector/opentelemetry-collector/service/command_build_info_test.go:71 +0xc85
  testing.tRunner()
      /opt/hostedtoolcache/go/1.18.7/x64/src/testing/testing.go:1439 +0x213
  testing.(*T).Run.func1()
      /opt/hostedtoolcache/go/1.18.7/x64/src/testing/testing.go:1486 +0x47

Goroutine 42 (running) created at:
  testing.(*T).Run()
      /opt/hostedtoolcache/go/1.18.7/x64/src/testing/testing.go:1486 +0x724
  testing.runTests.func1()
      /opt/hostedtoolcache/go/1.18.7/x64/src/testing/testing.go:1839 +0x99
  testing.tRunner()
      /opt/hostedtoolcache/go/1.18.7/x64/src/testing/testing.go:1439 +0x213
  testing.runTests()
      /opt/hostedtoolcache/go/1.18.7/x64/src/testing/testing.go:1837 +0x7e4
  testing.(*M).Run()
      /opt/hostedtoolcache/go/1.18.7/x64/src/testing/testing.go:1719 +0xa71
  main.main()
      _testmain.go:113 +0x2e4
==================

Chinwendu20 · 2022-11-15T17:41:52Z

Wow that is new. I would look into it

Signed-off-by: Maureen <amaka013@gmail.com>

Chinwendu20 · 2022-11-17T03:54:32Z

Thank you so much @codeboten. I have pushed a commit that should fix the error that you referenced but what I commented about the command, "make test" still stands, please look at my most recent screenshot. The codecov test also fails but I have written a test for my feature, any advice?

codeboten · 2022-11-21T19:50:48Z

@Chinwendu20 I've submitted a change to your PR to use cmd.OutOrStdout to print to stdout. It allows us to use the cmd.SetOut() functionality in cobra to simplify the test and avoid the race condition. Please take a look!

Chinwendu20 · 2022-11-22T05:41:01Z

Thank you. The unit tests are passing now. I will fix the lint test

Signed-off-by: Maureen <amaka013@gmail.com>

.chloggen/build-info-flag.yaml

codeboten

Thanks for all the work @Chinwendu20!

Bit of cleanup left over from open-telemetry#6322.

Bit of cleanup left over from #6322.

Chinwendu20 · 2022-12-03T09:33:14Z

Hello, please I would like to push a commit updating the readme with the correct command name and I would also like to update the file name of the command

jpkrohling · 2022-12-05T11:52:20Z

@Chinwendu20, please open a new PR with the new commit.

Chinwendu20 · 2022-12-07T17:49:26Z

Okay thank you

Enhancement: Added build command to output components in collector distribution in YAML format Signed-off-by: Maureen <amaka013@gmail.com> Co-authored-by: Juraci Paixão Kröhling <juraci.github@kroehling.de> Co-authored-by: Alex Boten <aboten@lightstep.com>

Bit of cleanup left over from open-telemetry#6322.

Enhancement: Added build command to output components in collector distribution in YAML format Signed-off-by: Maureen <amaka013@gmail.com> Co-authored-by: Juraci Paixão Kröhling <juraci.github@kroehling.de> Co-authored-by: Alex Boten <aboten@lightstep.com>

Bit of cleanup left over from open-telemetry#6322.

Chinwendu20 requested review from a team and bogdandrutu October 15, 2022 13:53

Chinwendu20 force-pushed the build-info-flag branch from 0f14527 to 73be175 Compare October 15, 2022 13:58

Created a fix for open-telemetry#4671

f0c3ead

Signed-off-by: Maureen <amaka013@gmail.com>

Chinwendu20 force-pushed the build-info-flag branch from 240fa64 to f0c3ead Compare October 15, 2022 18:27

codeboten reviewed Oct 17, 2022

View reviewed changes

service/flags.go Outdated Show resolved Hide resolved

Chinwendu20 and others added 2 commits October 18, 2022 08:24

Added unit test and file in chloggen

f459bbc

Signed-off-by: Maureen <amaka013@gmail.com>

Merge branch 'main' into build-info-flag

2ce77ed

jpkrohling self-requested a review October 18, 2022 20:11

jpkrohling reviewed Oct 19, 2022

View reviewed changes

jpkrohling changed the title ~~Fixes #4671~~ Add "build-info" flag Oct 19, 2022

Aneurysm9 reviewed Oct 19, 2022

View reviewed changes

service/command.go Outdated Show resolved Hide resolved

Update service/flags.go

37eb34d

Co-authored-by: Juraci Paixão Kröhling <juraci.github@kroehling.de>

Chinwendu20 added 3 commits October 20, 2022 00:43

Remove whitespace

363bca9

Signed-off-by: Maureen <amaka013@gmail.com>

Added build info sub command

8cd528a

Signed-off-by: Maureen <amaka013@gmail.com>

fix merge conflict

72da9b6

Signed-off-by: Maureen <amaka013@gmail.com>

Added test

03fa3d5

Signed-off-by: Maureen <amaka013@gmail.com>

Chinwendu20 changed the title ~~Add "build-info" flag~~ Add "build-info" Command Oct 20, 2022

Added comments in test

df35afd

Signed-off-by: Maureen <amaka013@gmail.com>

jpkrohling approved these changes Oct 20, 2022

View reviewed changes

Merge branch 'main' into build-info-flag

c9be8b7

Repalced deprecated config.Type with component.Type

42ae192

Signed-off-by: Maureen <amaka013@gmail.com>

bogdandrutu mentioned this pull request Nov 10, 2022

Created dry run flag for collector #6445

Closed

codeboten reviewed Nov 14, 2022

View reviewed changes

Fixed build info test

75cd2e4

Signed-off-by: Maureen <amaka013@gmail.com>

use cmd.OutOrStdout for output, makes testing easier

bcd3003

Fixed lint test

c3aad30

Signed-off-by: Maureen <amaka013@gmail.com>

codeboten approved these changes Nov 25, 2022

View reviewed changes

codeboten reviewed Nov 25, 2022

View reviewed changes

.chloggen/build-info-flag.yaml Outdated Show resolved Hide resolved

Update .chloggen/build-info-flag.yaml

b2bba0b

codeboten changed the title ~~Add "build-info" Command~~ Add "components" Command Nov 28, 2022

codeboten approved these changes Nov 28, 2022

View reviewed changes

codeboten merged commit dbe9e67 into open-telemetry:main Nov 28, 2022

codeboten pushed a commit to codeboten/opentelemetry-collector that referenced this pull request Nov 28, 2022

[chore] rename files to match command

a201028

Bit of cleanup left over from open-telemetry#6322.

codeboten mentioned this pull request Nov 28, 2022

[chore] rename files to match command #6628

Merged

codeboten pushed a commit that referenced this pull request Nov 28, 2022

[chore] rename files to match command (#6628)

a8645ad

Bit of cleanup left over from #6322.

Chinwendu20 mentioned this pull request Dec 3, 2022

Document the new sub command "components" open-telemetry/opentelemetry.io#2072

Closed

Chinwendu20 mentioned this pull request Dec 13, 2022

Removed error in service package readme, replaced ./otelcol build-info… #6770

Merged

jaronoff97 pushed a commit to lightstep/opentelemetry-collector that referenced this pull request Dec 14, 2022

[chore] rename files to match command (open-telemetry#6628)

14f437c

Bit of cleanup left over from open-telemetry#6322.

jaronoff97 pushed a commit to lightstep/opentelemetry-collector that referenced this pull request Dec 14, 2022

[chore] rename files to match command (open-telemetry#6628)

7629ff8

Bit of cleanup left over from open-telemetry#6322.

		components.Version = set.BuildInfo.Version

		yamlData, err := yaml.Marshal(components)

Add "components" Command #6322

Add "components" Command #6322

Conversation

Chinwendu20 commented Oct 15, 2022 • edited Loading

Chinwendu20 commented Oct 15, 2022

Chinwendu20 commented Oct 15, 2022 • edited Loading

codeboten left a comment

Choose a reason for hiding this comment

codecov bot commented Oct 17, 2022 • edited Loading

Codecov Report

Chinwendu20 commented Oct 18, 2022

jpkrohling left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Chinwendu20 Oct 19, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Chinwendu20 commented Oct 19, 2022

jpkrohling commented Oct 19, 2022

Chinwendu20 commented Oct 19, 2022

Chinwendu20 commented Oct 20, 2022

Chinwendu20 commented Oct 20, 2022

Chinwendu20 commented Oct 20, 2022 • edited Loading

jpkrohling left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Chinwendu20 commented Nov 8, 2022

codeboten left a comment

Choose a reason for hiding this comment

Chinwendu20 commented Nov 15, 2022

Chinwendu20 commented Nov 17, 2022

codeboten commented Nov 21, 2022

Chinwendu20 commented Nov 22, 2022

codeboten left a comment

Choose a reason for hiding this comment

Chinwendu20 commented Dec 3, 2022

jpkrohling commented Dec 5, 2022

Chinwendu20 commented Dec 7, 2022

Chinwendu20 commented Oct 15, 2022 •

edited

Loading

Chinwendu20 commented Oct 15, 2022 •

edited

Loading

codecov bot commented Oct 17, 2022 •

edited

Loading

Chinwendu20 Oct 19, 2022 •

edited

Loading

Chinwendu20 commented Oct 20, 2022 •

edited

Loading

jpkrohling left a comment •

edited

Loading