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

Too many recursive expansions #10617

Closed
danelson opened this issue Jul 15, 2024 · 3 comments · Fixed by #10712
Closed

Too many recursive expansions #10617

danelson opened this issue Jul 15, 2024 · 3 comments · Fixed by #10712
Labels
bug Something isn't working

Comments

@danelson
Copy link

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.

@danelson danelson added the bug Something isn't working label Jul 15, 2024
@mx-psi
Copy link
Member

mx-psi commented Jul 16, 2024

@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
Copy link
Author

@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
Copy link
Member

@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.

mx-psi pushed a commit that referenced this issue Jul 24, 2024
#### Description
Bumps the limit of how much recursion we allow. This check is also
gating non-recursive expansions. We probably could separate these
concerns, but thats work that isn't really worthwhile, a simple constant
bump is simple and will cover most users. For the rest, there are
workarounds.

<!-- Issue number if applicable -->
#### Link to tracking issue
Fixes
#10617
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants