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

Add "components" Command #6322

Merged
merged 25 commits into from
Nov 28, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
f0c3ead
Created a fix for #4671
Chinwendu20 Oct 15, 2022
f459bbc
Added unit test and file in chloggen
Chinwendu20 Oct 18, 2022
2ce77ed
Merge branch 'main' into build-info-flag
Chinwendu20 Oct 18, 2022
37eb34d
Update service/flags.go
Chinwendu20 Oct 19, 2022
363bca9
Remove whitespace
Chinwendu20 Oct 19, 2022
8cd528a
Added build info sub command
Chinwendu20 Oct 20, 2022
72da9b6
fix merge conflict
Chinwendu20 Oct 20, 2022
03fa3d5
Added test
Chinwendu20 Oct 20, 2022
df35afd
Added comments in test
Chinwendu20 Oct 20, 2022
560d5c1
Fixed CI errors
Chinwendu20 Oct 21, 2022
0084318
Separated new command test in separate file
Chinwendu20 Oct 25, 2022
cd495bf
Update service/README.md
codeboten Oct 26, 2022
6f9c455
Update service/command_build_info.go
codeboten Oct 26, 2022
80551fd
Corrected wrong values chlog file
Chinwendu20 Oct 27, 2022
3b69252
Apply suggestions from code review
codeboten Oct 27, 2022
cfc13a2
Merge branch 'build-info-flag' of https://github.com/Chinwendu20/open…
Chinwendu20 Oct 28, 2022
76b26e9
Added license
Chinwendu20 Nov 5, 2022
b6583e4
Changed command name to components
Chinwendu20 Nov 5, 2022
6a3cbe1
vanity import
Chinwendu20 Nov 7, 2022
c9be8b7
Merge branch 'main' into build-info-flag
Chinwendu20 Nov 8, 2022
42ae192
Repalced deprecated config.Type with component.Type
Chinwendu20 Nov 8, 2022
75cd2e4
Fixed build info test
Chinwendu20 Nov 17, 2022
bcd3003
use cmd.OutOrStdout for output, makes testing easier
Nov 21, 2022
c3aad30
Fixed lint test
Chinwendu20 Nov 22, 2022
b2bba0b
Update .chloggen/build-info-flag.yaml
codeboten Nov 25, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .chloggen/build-info-flag.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: enhancement

# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
component: build-info flag

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Added build flag to output components in collector distribution in YAML format

# One or more tracking issues or pull requests related to the change
issues: [#4671]
codeboten marked this conversation as resolved.
Show resolved Hide resolved

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:
1 change: 0 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@
- Add prometheus metric prefix to Collector's own telemetry when using OpenTelemetry for internal telemetry (#6223)
- `exporter/logging`: Apply consistent rendering of map values (#6244)
- Add support in the confmap.Resolver to expand embedded config URIs inside configuration. (#6276)

Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't change. Please undo it if you can.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I would revert this to its initial state before my commits.

### 🧰 Bug fixes 🧰

- Fixed bug where `telemetryInitializer` is not cleaned up when `newService` errors (#6239)
Expand Down
30 changes: 29 additions & 1 deletion service/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,33 @@ For more technical details about how configuration is resolved you can read the

`./otelcorecol --config=file:examples/local/otel-config.yaml --config="yaml:exporters::logging::loglevel: info"`


# How to check components available in a distribution
codeboten marked this conversation as resolved.
Show resolved Hide resolved

Use the flag --build-info. Below is an example:

```bash
.\otelcol --build-info
```
Sample output:

```yaml

version: 0.62.1-dev
receivers:
- otlp
processors:
- memory_limiter
- batch
exporters:
- logging
- otlp
- otlphttp
extensions:
- memory_ballast
- zpages

```
## How to override config properties?

The `--set` flag allows to set arbitrary config property. The `--set` values are merged into the final configuration
Expand Down Expand Up @@ -94,4 +121,5 @@ key:

1. Does not support setting a key that contains a dot `.`.
2. Does not support setting a key that contains a equal sign `=`.
3. The configuration key separator inside the value part of the property is "::". For example `--set "name={a::b: c}"` is equivalent with `--set name.a.b=c`.
3. The configuration key separator inside the value part of the property is "::". For example `--set "name={a::b: c}"` is equivalent with `--set name.a.b=c`.

Copy link
Member

Choose a reason for hiding this comment

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

What changed here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I would remove the whitespace

6 changes: 5 additions & 1 deletion service/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package service // import "go.opentelemetry.io/collector/service"

import (
"errors"

"github.com/spf13/cobra"

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

It does

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

"go.opentelemetry.io/collector/featuregate"
Expand All @@ -33,6 +32,11 @@ func NewCommand(set CollectorSettings) *cobra.Command {
if err := featuregate.GetRegistry().Apply(gatesList); err != nil {
return err
}
if BuildFlag {
err, _ := getBuildInfo(set)
return err
Aneurysm9 marked this conversation as resolved.
Show resolved Hide resolved
}

if set.ConfigProvider == nil {
var err error

Expand Down
52 changes: 49 additions & 3 deletions service/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,25 +17,41 @@ package service // import "go.opentelemetry.io/collector/service"
import (
"errors"
"flag"
"strings"

"fmt"
"go.opentelemetry.io/collector/config"
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ran gofmt -w . on the project. This is how it looks:
image
Should I look up correct import order and implement changes manually if this is not in line?

Copy link
Member

Choose a reason for hiding this comment

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

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

"go.opentelemetry.io/collector/featuregate"
"gopkg.in/yaml.v3"
"strings"
)

const (
configFlag = "config"
configFlag = "config"
buildInfoFlag = "build-info"
)

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

Choose a reason for hiding this comment

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

This should not be public

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, thanks. I would be changing that.

)

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

type componentsOutput struct {
Version string

Copy link
Member

Choose a reason for hiding this comment

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

Can you remove the extraneous empty lines?

Copy link
Contributor Author

@Chinwendu20 Chinwendu20 Oct 19, 2022

Choose a reason for hiding this comment

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

Ok, changing that. Thanks

Receivers []config.Type

Processors []config.Type

Exporters []config.Type

Extensions []config.Type
}

func (s *configFlagValue) Set(val string) error {
s.values = append(s.values, val)
return nil
Expand Down Expand Up @@ -70,10 +86,40 @@ func flags() *flag.FlagSet {
"feature-gates",
"Comma-delimited list of feature gate identifiers. Prefix with '-' to disable the feature. '+' or no prefix will enable the feature.")

flagSet.BoolVar(&BuildFlag, buildInfoFlag, false,
"Displays list of components available in collector distribution in yaml format",
Chinwendu20 marked this conversation as resolved.
Show resolved Hide resolved
)

return flagSet
}

func getConfigFlag(flagSet *flag.FlagSet) []string {
cfv := flagSet.Lookup(configFlag).Value.(*configFlagValue)
return append(cfv.values, cfv.sets...)
}

func getBuildInfo(set CollectorSettings) (error, []byte) {
components := componentsOutput{}
for ext, _ := range set.Factories.Extensions {
components.Extensions = append(components.Extensions, ext)
}
for prs, _ := range set.Factories.Processors {
components.Processors = append(components.Processors, prs)
}
for rcv, _ := range set.Factories.Receivers {
components.Receivers = append(components.Receivers, rcv)
}
for exp, _ := range set.Factories.Exporters {
components.Exporters = append(components.Exporters, exp)
}
components.Version = set.BuildInfo.Version

yamlData, err := yaml.Marshal(components)

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, thank you. On it.

if err != nil {
return err, nil
}

fmt.Println(string(yamlData))
codeboten marked this conversation as resolved.
Show resolved Hide resolved
return nil, yamlData
}
41 changes: 41 additions & 0 deletions service/flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@
package service

import (
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/component/componenttest"
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/featuregate"
"gopkg.in/yaml.v3"
"path/filepath"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -78,3 +84,38 @@ func TestSetFlag(t *testing.T) {
})
}
}
func TestBuildInfoFlag(t *testing.T) {
factories, err := componenttest.NopFactories()
require.NoError(t, err)

cfgProvider, err := NewConfigProvider(newDefaultConfigProviderSettings([]string{filepath.Join("testdata", "otelcol-nop.yaml")}))
require.NoError(t, err)

set := CollectorSettings{
BuildInfo: component.NewDefaultBuildInfo(),
Factories: factories,
ConfigProvider: cfgProvider,
telemetry: newColTelemetry(featuregate.NewRegistry()),
}
ExpectedYamlStruct := componentsOutput{
Version: "latest",

Copy link
Member

Choose a reason for hiding this comment

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

Reconsider the empty lines here as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok thank you

Receivers: []config.Type{"nop"},

Processors: []config.Type{"nop"},

Exporters: []config.Type{"nop"},

Extensions: []config.Type{"nop"},
}
ExpectedOutput, err := yaml.Marshal(ExpectedYamlStruct)

require.NoError(t, err)

err, Output := getBuildInfo(set)

require.NoError(t, err)

assert.Equal(t, ExpectedOutput, Output)

}