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(analytics): add missing usage events causing warning in logs #7109

Merged
merged 3 commits into from
Jan 24, 2023

Conversation

anshbansal
Copy link
Collaborator

@anshbansal anshbansal commented Jan 23, 2023

Not having this was causing logs like this in gms which caused confusion.

INFO  c.l.m.k.t.DataHubUsageEventTransformer:74 - Invalid event type: HomePageViewEvent
WARN  c.l.m.k.DataHubUsageEventsProcessor:56 - Failed to apply usage events transform to record: ...

Checklist

  • The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
  • Links to related issues (if applicable)
  • Tests for the changes have been added/updated (if applicable)
  • Docs related to the changes have been added/updated (if applicable). If a new feature has been added a Usage Guide has been added for the same.
  • For any breaking change/potential downtime/deprecation/big changes an entry has been made in Updating DataHub

@github-actions github-actions bot added devops PR or Issue related to DataHub backend & deployment product PR or Issue related to the DataHub UI/UX labels Jan 23, 2023
@anshbansal
Copy link
Collaborator Author

Expected to fail with ManuallyCreateLineageEvent being missing

@anshbansal
Copy link
Collaborator Author

Code check did fail https://github.com/datahub-project/datahub/actions/runs/3989112410/jobs/6841138499 with

Missing {'ManuallyCreateLineageEvent'} from DataHubUsageEventType.java. Please add

Added it now. So should pass now.

@anshbansal
Copy link
Collaborator Author

@anshbansal anshbansal changed the title fix(analytics): add missing analytics events fix(analytics): add missing usage events causing warning in logs Jan 23, 2023

ts_events_not_in_java = ts_events.difference(java_events)
if len(ts_events_not_in_java) > 0:
print(f"Missing {ts_events_not_in_java} from DataHubUsageEventType.java. Please add")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for adding this. We will need to watch these usage indexes as they continue to grow with all of these new events!

Overall, I like the idea of keeping these in sync.

Copy link
Collaborator

@jjoyce0510 jjoyce0510 left a comment

Choose a reason for hiding this comment

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

I'm looking forward to this change! Let's change from draft state when you're ready.

@anshbansal anshbansal marked this pull request as ready for review January 24, 2023 11:44
@anshbansal
Copy link
Collaborator Author

Tested on local and don't see the warning showing up anymore in gms logs.

@anshbansal
Copy link
Collaborator Author

Work on fixing the failing cypress is being done in #7116

@anshbansal anshbansal merged commit db96849 into master Jan 24, 2023
@anshbansal anshbansal deleted the aseem-fix-missing-usage-events branch January 24, 2023 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devops PR or Issue related to DataHub backend & deployment product PR or Issue related to the DataHub UI/UX
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants