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

fix cloudwatch-input query generation #10123

Closed
wants to merge 1 commit into from
Closed

fix cloudwatch-input query generation #10123

wants to merge 1 commit into from

Conversation

maxmoehl
Copy link

@maxmoehl maxmoehl commented Nov 18, 2021

Required for all PRs:

  • Updated associated README.md. (not applicable, or?)
  • Wrote appropriate unit tests.
  • Pull request title or commits are in conventional commit format

Resolves: #10122

getDataQueries iterates over all filteredMetrics and takes the address of the metric from that list to store it in the dataQueries map. However since go seems to re-use the same object for every iteration of the loop the pointer that is taken always points to the exact same memory location. Due to this the Metric field will always contain the same pointer (and therefore value) after the for loop is done. The fix is easy and I will provide it as soon as I am done with this issue: the metric struct needs to be copied once to allocate new memory, after that the address can be taken.

@telegraf-tiger
Copy link
Contributor

Thanks so much for the pull request!
🤝 ✒️ Just a reminder that the CLA has not yet been signed, and we'll need it before merging. Please sign the CLA when you get a chance, then post a comment here saying !signed-cla

@telegraf-tiger telegraf-tiger bot added the fix pr to fix corresponding bug label Nov 18, 2021
@powersj
Copy link
Contributor

powersj commented Nov 24, 2021

!signed-cla

@telegraf-tiger
Copy link
Contributor

Thanks so much for the pull request!
🤝 ✒️ Just a reminder that the CLA has not yet been signed, and we'll need it before merging. Please sign the CLA when you get a chance, then post a comment here saying !signed-cla

@powersj
Copy link
Contributor

powersj commented Nov 24, 2021

@maxmoehl I would love to see this merged next week before we cut 1.21. Can you look at signing the CLA please?

@maxmoehl
Copy link
Author

@powersj I'm currently in the process of getting the CLA approved by my employer, I will try to get it done in time. I'm also fine if #10112 gets merged and this one will be discarded, we just need this to be fixed soon :)

@powersj
Copy link
Contributor

powersj commented Dec 1, 2021

@maxmoehl thanks for this. In order to get this in before our next release, I took the PR in #10112. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix pr to fix corresponding bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[cloudwatch-input] dataQuery generation always returns the same dimension for a metric
2 participants