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 event tracking for project parse/load time #2893

Merged
merged 4 commits into from
Nov 19, 2020

Conversation

kwigley
Copy link
Contributor

@kwigley kwigley commented Nov 17, 2020

resolves #2823

Description

Added new event named load_project. The event includes high level tracking of project parse times. This will help track and inform efforts to increase dbt performance 👍

I decided to move the timing info into a dataclass to help me out a little when creating the event context. I can see us surfacing this info elsewhere as well.

Checklist

  • I have signed the CLA
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have updated the CHANGELOG.md and added information about my change to the "dbt next" section.

Copy link
Contributor

@jtcohen6 jtcohen6 left a comment

Choose a reason for hiding this comment

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

I've been testing this out, and it's looking great to me! Excited to dig into the data.

I don't think the event fires for the actual dbt parse command, which feels right—it's not really an invocation, and the explicit purpose of that command is to collect more detailed performance info than what's available here.

I forgot our convention of sticking the invocation_id in the structured event label. That's ok, I'm happy we're sticking it there and right in the context as well, and I don't mind the duplication.

@kwigley kwigley marked this pull request as ready for review November 19, 2020 00:28
@kwigley
Copy link
Contributor Author

kwigley commented Nov 19, 2020

@jtcohen6 awesome! I consciously didn't add the event to the parse task, so that's good to hear :) I'll note deets like this in the PR desc going forward.

Ya, I didn't realize the label convention until after we merged the event schema. We can always clean up if we ever iterate on it 👍

@kwigley kwigley self-assigned this Nov 19, 2020
@kwigley kwigley merged commit c19125b into dev/kiyoshi-kuromiya Nov 19, 2020
@kwigley kwigley deleted the feature/track-parse-time branch November 19, 2020 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add detailed tracking for start-of-invocation parse time
3 participants