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

No confmap support in releases #29345

Closed
gillg opened this issue Nov 20, 2023 · 19 comments
Closed

No confmap support in releases #29345

gillg opened this issue Nov 20, 2023 · 19 comments

Comments

@gillg
Copy link
Contributor

gillg commented Nov 20, 2023

Component(s)

confmap/provider/s3provider

What happened?

Description

Correct me if I'm wrong, but I have the feeling since these manifest has been introduced https://github.com/open-telemetry/opentelemetry-collector-releases/blob/main/distributions/otelcol/manifest.yaml & https://github.com/open-telemetry/opentelemetry-collector-releases/blob/main/distributions/otelcol-contrib/manifest.yaml the confmap https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/confmap/provider/s3provider seems not usable at all. Probably others too.
I always have an error "unsupported scheme", I also tried the feature flag which is deprectated.

Steps to Reproduce

Just try --config s3://xxxxxxx/xxxx.yml

Expected Result

Actual Result

Feature gate "confmap.expandEnabled" is stable and already enabled. It will be removed in version 0.75.0 and continued use of the gate after version 0.75.0 will result in an error.
Error: unsupported scheme on URI "s3://xxxxxxxxxx.s3.eu-west-3.amazonaws.com/otel-config/generic.yml"
2023/11/20 07:53:42 collector server run finished with error: unsupported scheme on URI "s3://xxxxxxxxxx.s3.eu-west-3.amazonaws.com/otel-config/generic.yml"

Collector version

0.89.0

Environment information

Environment

OS: al2023 ARM64

OpenTelemetry Collector configuration

N/A

Log output

N/A

Additional context

N/A

@gillg gillg added bug Something isn't working needs triage New item requiring triage labels Nov 20, 2023
Copy link
Contributor

Pinging code owners:

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

@crobert-1
Copy link
Member

Debugging

Reproduces everytime, here's the call stack I'm getting from hitting in a debugger:

confmap.NewResolver (resolver.go:95) go.opentelemetry.io/collector/confmap
otelcol.NewConfigProvider (configprovider.go:86) go.opentelemetry.io/collector/otelcol
otelcol.newCollectorWithFlags (command.go:44) go.opentelemetry.io/collector/otelcol
otelcol.NewCommand.func1 (command.go:23) go.opentelemetry.io/collector/otelcol
cobra.(*Command).execute (command.go:983) github.com/spf13/cobra
cobra.(*Command).ExecuteC (command.go:1115) github.com/spf13/cobra
cobra.(*Command).Execute (command.go:1039) github.com/spf13/cobra
main.runInteractive (main.go:27) main
main.run (main_others.go:11) main
main.main (main.go:20) main
runtime.main (proc.go:267) runtime
runtime.goexit (asm_amd64.s:1650) runtime

It's actually independent of the format of the URI. This is being hit in collector startup.

From the call stack line:

otelcol.newCollectorWithFlags (command.go:44) go.opentelemetry.io/collector/otelcol

There's a method call here from earlier in the function:

		set.ConfigProvider, err = NewConfigProvider(newDefaultConfigProviderSettings(configFlags))

Which calls into here:

func newDefaultConfigProviderSettings(uris []string) ConfigProviderSettings {
	return ConfigProviderSettings{
		ResolverSettings: confmap.ResolverSettings{
			URIs:       uris,
			Providers:  makeMapProvidersMap(fileprovider.New(), envprovider.New(), yamlprovider.New(), httpprovider.New(), httpsprovider.New()),
			Converters: []confmap.Converter{expandconverter.New()},
		},
	}
}

When we're checking for the scheme s3 it checks against this list of providers. Since the s3 provider is not listed here, the collector is failing to find a provider.

Conclusion

Running the contrib collector distribution and simply specifying a config path starting with s3 fails to find the s3 config provider. Is there more configuration required here to get the s3 provider added to the list map of valid schemes and scheme providers? Or is this a valid bug that needs addressed?

@crobert-1
Copy link
Member

It looks like it's simply not supported in contrib or core. Other distributions support it by manually adding it to the list of valid providers. I found an AWS support document referencing this, and found that the AWS distribution does this.

@Aneurysm9: Should we be adding support for the s3 provider to core/contrib? This may require moving the s3 provider to core, and enabling it by default.

Feel free to correct me if I'm wrong here, I may be misunderstanding something simple here.

@bryan-aguilar
Copy link
Contributor

From what I can tell so far your analysis looks to be correct @crobert-1.

Should we be adding support for the s3 provider to core/contrib? This may require moving the s3 provider to core, and enabling it by default.

I think this can be a more general question. Should the contrib distibution support different config map providers?

If the answer is yes then I think this problem should be solved generically by expanding the ocb tool.

@crobert-1
Copy link
Member

Good point. It seems like config map providers may be a bit of a gray area in components. This is currently the only config map provider in contrib, but it has a label as well as code owners. However, there's no metadata.yaml file to specify stability level, and there's no comment about it being included in any distribution of the collector.

I guess it's up to code owners to know if this could be considered Alpha stability. If it is, it would be good to make this a more full-fledged component in contrib (with a metadata.yaml file) and then add it to the collector builder.

In my mind, if the component is entirely in contrib and more stable than Development, I don't see any reason why it shouldn't be included in a release. Anyone's welcome to correct me here, I may be missing something.

@bryan-aguilar
Copy link
Contributor

IMO, this should not be in development stability anymore and is most likely an oversight after original contribution. We (ADOT) already include this in our distribution and provide support for this.

If it is, it would be good to make this a more full-fledged component in contrib (with a metadata.yaml file) and then add it to the collector builder.

We should probably do this first, as I'm not sure if mdatagen supports confmap components yet. This may need to be captured in a separate issue first. I'll take a look at getting a metadata.yaml file in there.

@bryan-aguilar
Copy link
Contributor

Looks like I missed it on first pass but it already has a metadata.yaml file but stability level is not defined yet.

@crobert-1
Copy link
Member

Looks like I missed it on first pass but it already has a metadata.yaml file but stability level is not defined yet.

I totally missed it myself as well. I think it's because there's no header in the README, but that'll be solved by the issue you've filed 👍

@crobert-1
Copy link
Member

I'm going to remove needs triage as the plan is to add support for this in the contrib repo.

@crobert-1 crobert-1 removed the needs triage New item requiring triage label Dec 2, 2023
@bryan-aguilar
Copy link
Contributor

Agreed, I think we will also need to make changes to the opentelemetry collector builder to support this but I have not evaluated changes needed there.

Copy link
Contributor

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

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

@github-actions github-actions bot added the Stale label Jan 31, 2024
@crobert-1 crobert-1 removed the Stale label Jan 31, 2024
Copy link
Contributor

github-actions bot commented Apr 1, 2024

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

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

@github-actions github-actions bot added the Stale label Apr 1, 2024
Copy link
Contributor

This issue has been closed as inactive because it has been stale for 120 days with no activity.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale May 31, 2024
@gillg
Copy link
Contributor Author

gillg commented May 31, 2024

Someone to reopen?
Cc @atoulme

@crobert-1 crobert-1 reopened this May 31, 2024
@atoulme
Copy link
Contributor

atoulme commented May 31, 2024

I believe the collector supports providers now. I didn't try this yet.

@Keimille
Copy link

I believe the collector supports providers now. I didn't try this yet.

I am still having this issue on the latest version, unless I am missing something altogether.

@evan-bradley
Copy link
Contributor

evan-bradley commented Jul 25, 2024

There's still some prerequisite work to be done on mdatagen to promote the providers to alpha stability, but the S3 provider will be added to the contrib distro in this PR: open-telemetry/opentelemetry-collector-releases#546.

In the meantime, you can still add the provider to your OCB manifests if you are using a custom distro like this.

Copy link
Contributor

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

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

@github-actions github-actions bot added the Stale label Sep 24, 2024
Copy link
Contributor

This issue has been closed as inactive because it has been stale for 120 days with no activity.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants