Unable to kafka receiver username config value to "$ConnectionString" with confmap.unifyEnvVarExpansion feature gate enabled

[ben-childs-docusign]

Describe the bug

We have some config to connect to azure event hubs using the kafka receiver this requires us to set the user name to "$ConnectionString" per azure documentation:
https://learn.microsoft.com/en-us/azure/event-hubs/azure-event-hubs-kafka-overview#shared-access-signature-sas

Prior to this feature gate enabled we were able to do this by configuring the receiver as follows:

  config:
    receivers:
      kafka:
        auth:
          sasl:
            mechanism: PLAIN
            username: "$$ConnectionString"
          tls:
            insecure: false
        protocol_version: 1.0.0
        topic: metricstopic

With version 105 of the collector we get this error:

Error: failed to get config: cannot resolve the configuration: cannot convert the confmap.Conf: variable substitution using $VAR has been deprecated in favor of ${VAR} and ${env:VAR} - please update $ConnectionString or temporarily disable the confmap.unifyEnvVarExpansion feature gate
2024/07/23 21:58:26 collector server run finished with error: failed to get config: cannot resolve the configuration: cannot convert the confmap.Conf: variable substitution using $VAR has been deprecated in favor of ${VAR} and ${env:VAR} - please update $ConnectionString or temporarily disable the confmap.unifyEnvVarExpansion feature gate

Steps to reproduce

Configure kafka receiver as shown above (no need to even add it to a pipeline)

What did you expect to see?

The config is loaded successfully

What did you see instead?

The collector crashes on boot

What version did you use?

v0.105.0

What config did you use?

  config:
    receivers:
      kafka:
        auth:
          sasl:
            mechanism: PLAIN
            username: "$$ConnectionString"
          tls:
            insecure: false
        protocol_version: 1.0.0
        topic: metricstopic

Environment

Ubuntu 20.04 - go 1.21

Additional context

Does this regex need to be updated to allow for "$$" prefixed strings to be excluded? Or is there some other fix missing?

opentelemetry-collector/confmap/converter/expandconverter/expand.go

Line 94 in c239e73

var regex = regexp.MustCompile(fmt.Sprintf(`\$%s`, regexp.QuoteMeta(str)))

Note that this PR proposes to fix this problem but I should have that fix since I'm using version 105:
#10560

[TylerHelmuth]

You're right. This is happening because we're in a transition stage. Once the feature gate is removed you'll be able to do $$ConnectionString or $ConnectionString without issue. But for now it is still getting caught by the expand converter.

I need to investigate why expandconverter isn't handling escaping the $ like it should.

[TylerHelmuth]

Oh I see why this is happening. When we fixed #10560, the result is that when the feature gate is enabled, $$ escaping is moved ahead of the expandconverter (since soon the expandconverter won't exist some day). But then the resulting string that gets passed to the expand converter is now $ConnectionString, which looks like an env var to expand, and it hits the failure logic because of the feature gate.

Again, this problem won't exist once the feature gate is made stable. For now I believe you have 2 options:

1. Try $$$ConnectionString (hows that for a silly work around lol)
2. Hold off bumping the collector version until the feature gate is made stable and expand converter is removed.

[dmitryax]

I believe $$someword is the most common use case. So #10560 doesn't seem like fixed the issue for the most users. @TylerHelmuth can we do it in the expand converter instead?

[TylerHelmuth]

@dmitryax i can experiment moving the $$ escaping to after the converters are run for now.

[TylerHelmuth]

Actually that would just bring the other big back.

We need to think about this some more. This bug at least has a workaround while the other one didn't.

[dmitryax]

What's the other bug?

[TylerHelmuth]

You couldn't do $${1}

[dmitryax]

Why can't we do escaping for both cases?

[dmitryax]

I don't get why this part is needed https://github.com/open-telemetry/opentelemetry-collector/blob/main/confmap/resolver.go#L177-L181. The expandconverter is doing exactly the same. So we currently do the unescaping/expansion twice.

[dmitryax]

If I'm not missing anything, this PR should resolve the issues. As the next step, we probably need to move $$->$ from expand converter to the resolver. @TylerHelmuth PTAL

[ben-childs-docusign]

Thank you for the quick fix!