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

Add usage reporter to track feature flags #1661

Merged
merged 33 commits into from
May 23, 2022
Merged

Add usage reporter to track feature flags #1661

merged 33 commits into from
May 23, 2022

Conversation

marctc
Copy link
Contributor

@marctc marctc commented Apr 29, 2022

PR Description

This PR adds the ability to pass the usage-report flag to the Grafana Agent to send
usage report of the enabled feature flags. The usage of this optional and disabled by default.

If enabled, it will send hourly the enabled flags to grafana.com.

Which issue(s) this PR fixes

Fixes #1488

Notes to the Reviewer

I followed a similar approach taken by grafana/loki#5361. I skipped
the part of leader selection when send the usage. I wasn't sure if we needed this bit.
I placed the initialization of the reporter in entrypoint.,go in function Start. Not sure
if it should go there or somewhere else.

We can extend the reporter in the future to send any other kind of metrics.

PR Checklist

  • CHANGELOG updated
  • Documentation added
  • Tests updated

@marctc marctc requested a review from brendamuir as a code owner April 29, 2022 10:11
@marctc marctc requested a review from a team April 29, 2022 10:11
pkg/config/config.go Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Show resolved Hide resolved
docs/user/configuration/flags.md Outdated Show resolved Hide resolved
docs/user/configuration/flags.md Outdated Show resolved Hide resolved
docs/user/configuration/flags.md Outdated Show resolved Hide resolved
marctc and others added 3 commits May 2, 2022 11:23
Co-authored-by: brendamuir <100768211+brendamuir@users.noreply.github.com>
Co-authored-by: brendamuir <100768211+brendamuir@users.noreply.github.com>
Co-authored-by: brendamuir <100768211+brendamuir@users.noreply.github.com>
CHANGELOG.md Outdated Show resolved Hide resolved
pkg/usagestats/seed.go Outdated Show resolved Hide resolved
pkg/usagestats/stats.go Outdated Show resolved Hide resolved
@marctc marctc requested review from brendamuir and tpaschalis May 4, 2022 13:26
@marctc
Copy link
Contributor Author

marctc commented May 4, 2022

The usage of this optional and disabled by default.

Based on the past experiences of the Loki team, it's better to have this option
enabled by default as rarely users active this option.

pkg/config/config.go Outdated Show resolved Hide resolved
pkg/config/config.go Outdated Show resolved Hide resolved
marctc and others added 2 commits May 5, 2022 12:06
Co-authored-by: Paschalis Tsilias <tpaschalis@users.noreply.github.com>
Copy link
Member

@tpaschalis tpaschalis left a comment

Choose a reason for hiding this comment

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

I haven't tested this (especially on Windows), but it looks okay to me!

// The interval is based off the creation of the agent seed to avoid all agents reporting at the same time.
func nextReport(interval time.Duration, createdAt, now time.Time) time.Time {
// createdAt * (x * interval ) >= now
return createdAt.Add(time.Duration(math.Ceil(float64(now.Sub(createdAt))/float64(interval))) * interval)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe separate this into a few statements for readability

docs/user/configuration/flags.md Outdated Show resolved Hide resolved
docs/user/configuration/flags.md Outdated Show resolved Hide resolved
docs/user/configuration/flags.md Outdated Show resolved Hide resolved
marctc and others added 2 commits May 18, 2022 16:21
Co-authored-by: brendamuir <100768211+brendamuir@users.noreply.github.com>
Co-authored-by: brendamuir <100768211+brendamuir@users.noreply.github.com>
Co-authored-by: brendamuir <100768211+brendamuir@users.noreply.github.com>
Copy link
Collaborator

@mattdurham mattdurham left a comment

Choose a reason for hiding this comment

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

A few small non critical things but overall good

Copy link

@DanCech DanCech left a comment

Choose a reason for hiding this comment

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

nice

@mattdurham mattdurham merged commit 0abd5dd into main May 23, 2022
@mattdurham mattdurham deleted the features_flag branch May 23, 2022 15:43
@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 Mar 27, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add opt in feature telemetry
6 participants