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

Include Connectors when using Cfgmetadatagen #26990

Closed
BTripp1986 opened this issue Sep 18, 2023 · 4 comments · Fixed by #29001
Closed

Include Connectors when using Cfgmetadatagen #26990

BTripp1986 opened this issue Sep 18, 2023 · 4 comments · Fixed by #29001
Labels
cmd/configschema configschema command enhancement New feature or request

Comments

@BTripp1986
Copy link

Component(s)

cmd/configschema

Is your feature request related to a problem? Please describe.

As part of a tool have built I use the cfgmetadatagen to generate metadata for the component configs but it does not generate metadata for connectors. Most connectors don't require any additional config, but the countconnector and spanmetricsconnector do require additional config.

Describe the solution you'd like

I would like connectors to be included when generating metadata with the cfgmetadatagen.

Describe alternatives you've considered

I am currently using regex to parse the source files, a solution I was using for all components until I discovered the cfgmetadatagen. I am now refactoring my app to use cfgmetadatagen, but it would be great to have a consistent format from which to get config fields and default values.

Additional context

It seems that this would be a fairly trivial addition, but I could be wrong. I am a Go novice so I could potentially create a PR for this, but if this is deemed a valid feature request it would be great if someone more competent with Go could add the feature.

@BTripp1986 BTripp1986 added enhancement New feature or request needs triage New item requiring triage labels Sep 18, 2023
@github-actions github-actions bot added the cmd/configschema configschema command label Sep 18, 2023
@github-actions
Copy link
Contributor

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@mx-psi
Copy link
Member

mx-psi commented Sep 19, 2023

I am in favor of this

@haoqixu
Copy link
Member

haoqixu commented Nov 7, 2023

I have submitted a draft PR (#29001), but it turns out that configschema will return error on the forwardconnector.

forwardconnector returns an anonymous empty struct as default config and reflect.Type.PkgPaht() returns empty string for non-defined types. As a result, configschema failed to find package path for forwardconnector (via configschema.DirResolver.TypeToPackagePath()).

I think maybe we can treat non-defined types as empty config and skip the comment search for them. 🤔

@mx-psi
Copy link
Member

mx-psi commented Nov 7, 2023

I think maybe we can treat non-defined types as empty config and skip the comment search for them

I would be in favor of just fixing the forward connector by defining a

type Config struct {
}

which I think should fix things?

@mx-psi mx-psi closed this as completed Nov 7, 2023
@mx-psi mx-psi reopened this Nov 7, 2023
bogdandrutu pushed a commit to open-telemetry/opentelemetry-collector that referenced this issue Nov 8, 2023
**Description:**
`cmd/configschema` currently doesn't work with anonymous config struct.
(see
open-telemetry/opentelemetry-collector-contrib#26990 (comment))

This PR refactors `forwardconnector` to use a defined struct for config.
mx-psi added a commit that referenced this issue Nov 21, 2023
**Description:** include connectors

**Link to tracking Issue:** resolve #26990

---------

Co-authored-by: Pablo Baeyens <[email protected]>
RoryCrispin pushed a commit to ClickHouse/opentelemetry-collector-contrib that referenced this issue Nov 24, 2023
**Description:** include connectors

**Link to tracking Issue:** resolve open-telemetry#26990

---------

Co-authored-by: Pablo Baeyens <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmd/configschema configschema command enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants