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

Remove the quadruple dollar hack #357

Merged
merged 1 commit into from
Jan 15, 2022
Merged

Remove the quadruple dollar hack #357

merged 1 commit into from
Jan 15, 2022

Conversation

dmitryax
Copy link
Contributor

@dmitryax dmitryax commented Jan 14, 2022

Double expansion issue was fixed with the recent splunk-otel-collector upgrade. Now OTel collector configs are expanded only once and hack with $$$$ is not needed anymore.

  • temporary set the splunk-otel-collector docker image to the latest dev version to keep the main in a working state.

@rockb1017
Copy link
Contributor

I see that $$$$ is changed to $$.
but I think $$ should also be changed to $.

@dmitryax
Copy link
Contributor Author

dmitryax commented Jan 14, 2022

I see $$ only in places like $$.logtag which are supposed to work as expected and should stay the same. They didn't require $$$$ before because double expansion would be the following: $$.logtag -> $.logtag -> $.logtag, while double expansion of $$body: $$body -> $body -> "".

@rockb1017
Copy link
Contributor

oh hmm I see. thanks for explanation. that is why the CI pipeline worked. I was wondering.. LGTM!

@rockb1017
Copy link
Contributor

so it will work with just one $ for $.* ?

@dmitryax
Copy link
Contributor Author

so it will work with just one $ for $.* ?

It will, but I would leave it with $$ for consistency. Otherwise people may be confused when to use one and when two signs.

Double expansion issue was fixed with the recent splunk-otel-collector upgrade. Now OTel collector configs are expanded only once and hack with `$$$$` is not needed anymore.

+ temporary set splunk-otel-collector docker image to the latest dev version to keep the main in a working state.
@dmitryax dmitryax merged commit 5439a80 into main Jan 15, 2022
@dmitryax dmitryax deleted the dollar-dollar-fix branch January 17, 2022 04:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants