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

Aligned kedro-viz telemetry with kedro-telemetry #2112

Merged
merged 13 commits into from
Sep 30, 2024
Merged

Conversation

jitu5
Copy link
Contributor

@jitu5 jitu5 commented Sep 25, 2024

Description

Resolves #2020

This PR updates existing telemetry setup in Kedro-Viz:

  • Added kedro-telemetry as required dependency.
  • Used _check_for_telemetry_consent from kedro-telemetry
    • To check environment variables DO_NOT_TRACK or KEDRO_DISABLE_TELEMETRY
    • To check for consent in .telemetry file
  • Updated get_heap_identity to generate an unique UUID for each user of kedro-telemetry.
  • If kedro-telemetry is not installed, it should be treated as consent being False.

Heap event data: (Identity)
Before:
Screenshot 2024-09-25 at 10 53 46 a m

After:
Screenshot 2024-09-25 at 11 17 17 a m

QA notes

  • All test should pass
  • Screenshot 2024-09-25 at 7 30 10 p m

Checklist

  • Read the contributing guidelines
  • Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • Updated the documentation to reflect the code changes
  • Added new entries to the RELEASE.md file
  • Added tests to cover my changes

Signed-off-by: Jitendra Gundaniya <jitendra_gundaniya@mckinsey.com>
Signed-off-by: Jitendra Gundaniya <jitendra_gundaniya@mckinsey.com>
Signed-off-by: Jitendra Gundaniya <jitendra_gundaniya@mckinsey.com>
Signed-off-by: Jitendra Gundaniya <jitendra_gundaniya@mckinsey.com>
Signed-off-by: Jitendra Gundaniya <jitendra_gundaniya@mckinsey.com>
@jitu5 jitu5 requested a review from DimedS September 25, 2024 18:31
@jitu5 jitu5 self-assigned this Sep 25, 2024
@jitu5 jitu5 marked this pull request as ready for review September 25, 2024 18:33
Copy link

@DimedS DimedS left a comment

Choose a reason for hiding this comment

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

Thank you @jitu5, LGTM!

Copy link
Contributor

@ravi-kumar-pilla ravi-kumar-pilla 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 to me 💯 ... thank you @jitu5

Copy link
Contributor

@rashidakanchwala rashidakanchwala left a comment

Choose a reason for hiding this comment

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

thank you @jitu5 , Will have to connect with @merelcht to see if it makes sense to convert the functions to public since Kedro-viz accessed it.

Signed-off-by: Jitendra Gundaniya <jitendra_gundaniya@mckinsey.com>
Signed-off-by: Jitendra Gundaniya <jitendra_gundaniya@mckinsey.com>
Signed-off-by: Jitendra Gundaniya <jitendra_gundaniya@mckinsey.com>
Signed-off-by: Jitendra Gundaniya <jitendra_gundaniya@mckinsey.com>
Signed-off-by: Jitendra Gundaniya <jitendra_gundaniya@mckinsey.com>
Signed-off-by: Jitendra Gundaniya <jitendra_gundaniya@mckinsey.com>
@jitu5 jitu5 merged commit 363f0dd into main Sep 30, 2024
40 checks passed
@jitu5 jitu5 deleted the feature/viz_telemetry branch September 30, 2024 12:36
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.

Update kedro-viz telemetry to reflect recent changes in kedro-telemetry
4 participants