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: reduce telemetry events on listXXX events #776

Merged
merged 1 commit into from
Nov 21, 2022

Conversation

benoitf
Copy link
Collaborator

@benoitf benoitf commented Nov 4, 2022

What does this PR do?

It is sent only after 24h from previous event if user is keeping running the tool
if you restart it will send the event again

Screenshot/screencast of this PR

What issues does this PR fix or reference?

Fixes #658

How to test this PR?

Use a proxy and monitor telemetry events being sent

Change-Id: Id17b2131abcae17a5c9262240d0910c2ddca63c1
Signed-off-by: Florent Benoit fbenoit@redhat.com

It is sent only after 24h from previous event if user
 is keeping running the tool

Change-Id: Id17b2131abcae17a5c9262240d0910c2ddca63c1
Signed-off-by: Florent Benoit <fbenoit@redhat.com>
@cdrage
Copy link
Contributor

cdrage commented Nov 4, 2022

Unsure how to troubleshoot telemetry PR's, but does this LGTM @slemeur ?

@slemeur
Copy link
Collaborator

slemeur commented Nov 4, 2022

Instead of not sending the event - could we:

  • send the first one of every 24h
  • count the number of time the action occurs during the 24h interval ?

@benoitf
Copy link
Collaborator Author

benoitf commented Nov 4, 2022

@slemeur I'm not sure it's useful

@benoitf
Copy link
Collaborator Author

benoitf commented Nov 4, 2022

send the first one of every 24h

IMOH It doesn't make any sense. If you're not pulling/running containers, we would send the same event than before

count the number of time the action occurs during the 24h interval ?

list events are not triggered directly using user action (for example starting a new container is asking to list again the containers)

If you need something every 24h then it should be another report/event

@slemeur
Copy link
Collaborator

slemeur commented Nov 17, 2022

We should proceed with this proposition.
But I'd like that we merge it once we have #855 ready as well.

@benoitf benoitf requested review from cdrage and vzhukovs and removed request for vzhukovs and cdrage November 21, 2022 15:09
Copy link
Contributor

@cdrage cdrage left a comment

Choose a reason for hiding this comment

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

Code looks good (unsure how to test). But LGTM.

@cdrage cdrage merged commit 5f6e735 into containers:main Nov 21, 2022
@podman-desktop-bot podman-desktop-bot added this to the 0.10.0 milestone Nov 21, 2022
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.

Reduce telemetry volume
4 participants