open-telemetry / opentelemetry-collector-contrib

Contrib repository for the OpenTelemetry Collector
https://opentelemetry.io
Apache License 2.0
3.13k stars 2.41k forks source link

New component: AWS Secrets Manager Provider #19368

Closed driverpt closed 8 months ago

driverpt commented 1 year ago

The purpose and use-cases of the new component

Currently OTEL Collector in AWS is used in ECS, since ECS injects secrets directly from AWS Secrets Manager to Environment Variables. This forces us to attach AWS Lambdas in VPC's to be able to route all Telemetry to ECS Container running OTEL Collector.

For better resource usage, we would like to use OTEL Collector as Lambda Layer and inject the secrets, so that there's no need to attach to a specific VPC and potentially causing IP Exhaustion if you have a lot of invocations.

Example configuration for the component

receivers:
  otlp:
    protocols:
      grpc:
      http:

processors:
  batch:
  memory_limiter:

exporters:
  otlp/provider1:
    endpoint: https://provider1.domain.com:12345
    headers:
      api-key: ${secretsmanager:<SecretArnOrName>}
    compression: gzip

service:
  pipelines:
    traces:
      receivers: [otlp]
      processors: [memory_limiter, batch]
      exporters: [otlp/provider1]
    metrics:
      receivers: [otlp]
      processors: [memory_limiter, batch]
      exporters: [otlp/provider1]

Telemetry data types supported

N/A

Is this a vendor-specific component?

Sponsor (optional)

No response

Additional context

No response

driverpt commented 1 year ago

Upstream Link: https://github.com/aws-observability/aws-otel-collector/issues/1894

driverpt commented 1 year ago

@mhausenblas , do you want to Sponsor?

mhausenblas commented 1 year ago

@driverpt while I'm supportive, I technically can't sponsor it since I ain't no maintainer ;)

CC: @Aneurysm9

rapphil commented 1 year ago

For better resource usage, we would like to use OTEL Collector as Lambda Layer and inject the secrets, so that there's no need to attach to a specific VPC and potentially causing IP Exhaustion if you have a lot of invocations.

I'm not following this part. To inject secrets into environment variables in lambda, do you need to attach it to a VPC?

Have you considered this alternative? https://aws.amazon.com/blogs/compute/creating-aws-lambda-environmental-variables-from-aws-secrets-manager/

driverpt commented 1 year ago

Hello @rapphil , haven't tried that yet. Will make sure I give it a shot on Monday. But from a quick read through out the article, it seems that it only works for provided.al2 Runtimes.

Will give more feedback once I try it.

rapphil commented 1 year ago

Hello @driverpt , can you clarify the statement about IP Exhaustion if you have a lot of invocations? I'm still interested in that specific issue in the context of secrets manager.

driverpt commented 1 year ago

Hello @rapphil , we require SecretsManager to be able to Inject the External Observability Provider Key in OTEL Collector.

Currently on ECS we can use the Secrets part to inject it from SecretsManager.

This would allow us to have VPC-less Lambdas and be able to use OTEL Collector as a Layer instead of the need to attach Lambdas to VPC + OTEL Collector on ECS.

If we have a lot of concurrent Lambda Invocations we will exhaust the DHCP IP Pool

Aneurysm9 commented 1 year ago

I'm happy to sponsor this component, but I think I'd like it to be somewhat more general. Because each config provider needs a URI-like scheme identifier, and because most AWS resources can be referenced with URN-like ARNs, I'm thinking that an arn: config provider would be good to have. We could support Secrets Manager ARNs first and report an error for any other type. This would help avoid any naming concerns with secretsmanager: vs. awssecretsmanager: or any other scheme name as well as providing a clear path forward for expanding support to SSM Parameter Store or other AWS resources that could be used to store configuration.

driverpt commented 1 year ago

@Aneurysm9 , I wanted to create the PR with ${aws:secretsmanager}, but I was unsure about that convention. IMHO, given that we may have multiple providers, I think a "Namespace" should be reserved for the Cloud Provider, e.g.:

${gcp:secretsmanager:<value>}
${aws:secretsmanager:<arn or name>}
${digitalocean:secretsmanager:<value>}

WDYT ?

Aneurysm9 commented 1 year ago

I don't think that's a problem we need to solve at this point for adding AWS Secrets Manager support as we can use the ARN. Given that they have the form arn:aws:secretsmanager:<Region>:<AccountId>:secret:SecretName-6RandomCharacters we can extract all the information needed from the ARN.

driverpt commented 1 year ago

I don't think that's a problem we need to solve at this point for adding AWS Secrets Manager support as we can use the ARN. Given that they have the form arn:aws:secretsmanager:<Region>:<AccountId>:secret:SecretName-6RandomCharacters we can extract all the information needed from the ARN.

Partition is missing, arn:<Partition>:secretsmanager:<Region>:<AccountId>:secret:SecretName-6RandomCharacters

What do you suggest that I change in the PR I've created?

github-actions[bot] commented 1 year ago

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.

driverpt commented 1 year ago

No stale

github-actions[bot] commented 1 year ago

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.

driverpt commented 1 year ago

Ping

atoulme commented 1 year ago

I will sponsor this new component.

github-actions[bot] commented 12 months ago

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.

driverpt commented 11 months ago

Any ETA on this one?

atoulme commented 11 months ago

We need help to resolve the conflict on https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/19341 and we can then move forward.

github-actions[bot] commented 9 months ago

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.

atoulme commented 8 months ago

Completed!