open-telemetry / opentelemetry-collector

OpenTelemetry Collector
https://opentelemetry.io
Apache License 2.0
4.23k stars 1.4k forks source link

Too many recursive expansions #10617

Closed danelson closed 1 month ago

danelson commented 1 month ago

Describe the bug After updating to v0.104.0 I receive the following error.

failed to resolve config: cannot resolve the configuration: too many recursive expansions

https://github.com/open-telemetry/opentelemetry-collector/blob/main/confmap/expand.go#L27

I am not actually sure this is a bug but as it seems somewhat intentional based on the above link. It is at least a regression with the previous behavior.

Steps to reproduce Create a string in the collector config that contains more than 100 environment variables.

What did you expect to see? I expected it not to error.

What did you see instead? Collector fails to start with the following error:

failed to resolve config: cannot resolve the configuration: too many recursive expansions

What version did you use? v0.104.0

What config did you use?

receivers:
  otlp:
    protocols:
      grpc:
        endpoint: 0.0.0.0:4317
      http:
        endpoint: 0.0.0.0:4318
        cors:
          allowed_origins: ["*"]
          allowed_headers: ["*"]

processors:
  transform:
    error_mode: ignore
    log_statements:
      - context: log
        statements:
          - set(attributes["test"] = "${env:VARIABLE_001},${env:VARIABLE_002},${env:VARIABLE_003},${env:VARIABLE_004},${env:VARIABLE_005},${env:VARIABLE_006},${env:VARIABLE_007},${env:VARIABLE_008},${env:VARIABLE_009},${env:VARIABLE_010},${env:VARIABLE_011},${env:VARIABLE_012},${env:VARIABLE_013},${env:VARIABLE_014},${env:VARIABLE_015},${env:VARIABLE_016},${env:VARIABLE_017},${env:VARIABLE_018},${env:VARIABLE_019},${env:VARIABLE_020},${env:VARIABLE_021},${env:VARIABLE_022},${env:VARIABLE_023},${env:VARIABLE_024},${env:VARIABLE_025},${env:VARIABLE_026},${env:VARIABLE_027},${env:VARIABLE_028},${env:VARIABLE_029},${env:VARIABLE_030},${env:VARIABLE_031},${env:VARIABLE_032},${env:VARIABLE_033},${env:VARIABLE_034},${env:VARIABLE_035},${env:VARIABLE_036},${env:VARIABLE_037},${env:VARIABLE_038},${env:VARIABLE_039},${env:VARIABLE_040},${env:VARIABLE_041},${env:VARIABLE_042},${env:VARIABLE_043},${env:VARIABLE_044},${env:VARIABLE_045},${env:VARIABLE_046},${env:VARIABLE_047},${env:VARIABLE_048},${env:VARIABLE_049},${env:VARIABLE_050},${env:VARIABLE_051},${env:VARIABLE_052},${env:VARIABLE_053},${env:VARIABLE_054},${env:VARIABLE_055},${env:VARIABLE_056},${env:VARIABLE_057},${env:VARIABLE_058},${env:VARIABLE_059},${env:VARIABLE_060},${env:VARIABLE_061},${env:VARIABLE_062},${env:VARIABLE_063},${env:VARIABLE_064},${env:VARIABLE_065},${env:VARIABLE_066},${env:VARIABLE_067},${env:VARIABLE_068},${env:VARIABLE_069},${env:VARIABLE_070},${env:VARIABLE_071},${env:VARIABLE_072},${env:VARIABLE_073},${env:VARIABLE_074},${env:VARIABLE_075},${env:VARIABLE_076},${env:VARIABLE_077},${env:VARIABLE_078},${env:VARIABLE_079},${env:VARIABLE_080},${env:VARIABLE_081},${env:VARIABLE_082},${env:VARIABLE_083},${env:VARIABLE_084},${env:VARIABLE_085},${env:VARIABLE_086},${env:VARIABLE_087},${env:VARIABLE_088},${env:VARIABLE_089},${env:VARIABLE_090},${env:VARIABLE_091},${env:VARIABLE_092},${env:VARIABLE_093},${env:VARIABLE_094},${env:VARIABLE_095},${env:VARIABLE_096},${env:VARIABLE_097},${env:VARIABLE_098},${env:VARIABLE_099},${env:VARIABLE_100},${env:VARIABLE_101}")

exporters:
  debug:
    verbosity: detailed

service:
  pipelines:
    logs:
      receivers: [otlp]
      processors: [transform]
      exporters: [debug]

Environment otel/opentelemetry-collector-contrib:0.104.0 in docker

Additional context The above config is just an example. I have a custom component that loads ~110 environment variables (which will continue to grow) as a common separated list. I could modify this component to work differently but just wanted to make sure this was intended.

Disabling the feature flag confmap.unifyEnvVarExpansion allows the above config to be loaded successfully.

mx-psi commented 1 month ago

@danelson Hey, thanks for the issue! I would like to know more about your use case before deciding on what to do here. What are you trying to accomplish and what alternatives have you tried?

danelson commented 1 month ago

@mx-psi thanks for the response.

My component basically implements its own authentication and I load a bunch of tokens in via environment variables which I validate incoming headers against. I actually have a different flow where I do something similar via nginx so I am thinking that is a better approach that I can leverage. I just need to update some routing in my cluster.

I could also load the environment variables based on a convention (prefix) which would allow me to collapse this to a single environment variable.

I know there are some auth extensions (although I don't think any serve my use case). I could also explore writing my own.

So I know I have have some workarounds. Just wanted to make sure this was an intentional change since it was breaking from before.

TylerHelmuth commented 1 month ago

@danelson this is technically an unexpected consequence of making the env vars expanded by the envprovider instead of the expandconverter, but a true fix is probably too much work to be worth it. I think we'll just bump the recursive check constant to a higher number since this is a startup check and not hot-path execution and there are lots of work arounds like you mentioned.