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.

Sign up for GitHub

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

Jump to bottom

chore(faro): prefix measurement values when parsing faro measurements #6810

Merged
merged 5 commits into from
Jun 6, 2024
Merged

chore(faro): prefix measurement values when parsing faro measurements #6810

merged 5 commits into from
Jun 6, 2024

Conversation

codecapitano
Copy link
Contributor

@codecapitano codecapitano commented Apr 2, 2024 •
edited
Loading

PR Description

This PR aligns the hot to translate Faro measuerements with the latest Faro cloud receiver updates.

For reference have a look at the https://github.com/grafana/app-o11y-kwl-endpoint/pull/397.

Which issue(s) this PR fixes

Notes to the Reviewer

PR Checklist

  • CHANGELOG.md updated
  • Documentation added
  • Tests updated
  • Config converters updated

@codecapitano codecapitano self-assigned this Apr 2, 2024
@codecapitano codecapitano added enhancement New feature or request go Pull requests that update Go code labels Apr 2, 2024
@codecapitano codecapitano changed the title chore(faro): prefix measurement values chore(faro): prefix measurement values when parsing Faro measurements Apr 2, 2024
@codecapitano codecapitano changed the title chore(faro): prefix measurement values when parsing Faro measurements chore(faro): prefix measurement values when parsing faro measurements Apr 2, 2024
@codecapitano codecapitano force-pushed the faro_prefix-measurement-values branch from c79b1bf to 884d304 Compare April 5, 2024 07:38
wildum
wildum reviewed Apr 5, 2024
View reviewed changes
Copy link
Contributor

@wildum wildum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just a question on the string conversion and the duplicated entry in the changelog

CHANGELOG.md Outdated Show resolved Hide resolved
internal/component/faro/receiver/internal/payload/payload.go Outdated Show resolved Hide resolved
@codecapitano codecapitano force-pushed the faro_prefix-measurement-values branch from 1b2fcae to dcf10d1 Compare April 5, 2024 12:26
@rfratto rfratto added the variant/flow Relatd to Grafana Agent Flow. label Apr 9, 2024
@codecapitano
Copy link
Contributor Author

Hi @wildum 👋 I have no permission to merge the branch.
How's the process here, just ping the agent team or will automatically be merged at some time?

@wildum
Copy link
Contributor

wildum commented Apr 10, 2024

Hey sorry for dragging this up, we just made the Alloy repo public and we need to redirect the PRs there. We will only be merging bugfixes to the agent from now on.
We are just figuring out a process to make the transition smooth. You don't need to do anything, the PR will be merged to Alloy soon

@codecapitano
Copy link
Contributor Author

Hey sorry for dragging this up, we just made the Alloy repo public and we need to redirect the PRs there. We will only be merging bugfixes to the agent from now on. We are just figuring out a process to make the transition smooth. You don't need to do anything, the PR will be merged to Alloy soon

Great, thank you so much 🙏

@github-actions GitHub Actions
Copy link
Contributor

This PR has not had any activity in the past 30 days, so the needs-attention label has been added to it.
If you do not have enough time to follow up on this PR or you think it's no longer relevant, consider closing it.
The needs-attention label signals to maintainers that something has fallen through the cracks. No action is needed by you; your PR will be kept open and you do not have to respond to this comment. The label will be removed the next time this job runs if there is new activity.
Thank you for your contributions!

@github-actions github-actions bot added the needs-attention An issue or PR has been sitting around and needs attention. label May 16, 2024
@codecapitano
Copy link
Contributor Author

Hi @wildum any updates on this?
We have customers running into this issue.

@wildum
Copy link
Contributor

wildum commented Jun 6, 2024

Hey, super sorry we actually moved all the issues but not the PRs and I forgot about it. Actually this seems to be relevant for the agent as it looks more like a bugfix than an enhancement. I can merge this PR and create a new one in Alloy and get it merged. Is that ok?

@wildum wildum mentioned this pull request Jun 6, 2024
Merged
@wildum
Copy link
Contributor

wildum commented Jun 6, 2024

here is the alloy PR: grafana/alloy#991. I extended a unit test to check that the float values had the prefix

@wildum
Copy link
Contributor

wildum commented Jun 6, 2024

merged in Alloy, I will also merge it in a bit to the agent

@codecapitano
Copy link
Contributor Author

merged in Alloy, I will also merge it in a bit to the agent

Great thank you so much @wildum 🙏

@wildum wildum merged commit 53d022d into main Jun 6, 2024
10 checks passed
@wildum wildum deleted the faro_prefix-measurement-values branch June 6, 2024 09:44
@wildum
Copy link
Contributor

wildum commented Jun 6, 2024

All done, sorry again for the unnecessary delay!

@codecapitano
Copy link
Contributor Author

codecapitano commented Jun 6, 2024 •
edited
Loading

Thank you so much and no worries. I think your team had a ton of work around Alloy release time.
Ans I could've asked earlier as well.

@github-actions github-actions bot added the frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. label Jul 7, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@rlankfo rlankfo rlankfo left review comments

@wildum wildum wildum left review comments

@mar4uk mar4uk mar4uk approved these changes

Assignees

@codecapitano codecapitano

Labels
enhancement New feature or request frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. go Pull requests that update Go code needs-attention An issue or PR has been sitting around and needs attention. variant/flow Relatd to Grafana Agent Flow.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@codecapitano @wildum @rlankfo @mar4uk @rfratto