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

removes start event from telemetry #77

Merged
merged 4 commits into from
Oct 25, 2023
Merged

removes start event from telemetry #77

merged 4 commits into from
Oct 25, 2023

Conversation

edublancas
Copy link
Contributor

@edublancas edublancas commented Oct 24, 2023

Describe your changes

when recording telemetry events, we used to store a start and success/fail event. to reduce the number of events, we only trigger success/fail now.

the actual change is just a few lines of code, the rest are tests that had to be updated

once this is approved, I can make a new release

Issue number

Closes #X

Checklist before requesting a review


📚 Documentation preview 📚: https://ploomber-core--77.org.readthedocs.build/en/77/

@edublancas edublancas requested a review from idomic October 24, 2023 23:24
@edublancas edublancas marked this pull request as draft October 24, 2023 23:25
@edublancas edublancas marked this pull request as ready for review October 24, 2023 23:37
Copy link
Contributor

@idomic idomic left a comment

Choose a reason for hiding this comment

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

lg overall, the main thing is, I think we should track started - wdyt?
Then I think we'll have a full view, everyone that got in + the ones that errored out as part of the flow.

@@ -75,7 +75,7 @@ For more details, see the [API Reference](api/telemetry).

+++

To unit test decorated functions, call the function and check `__wrapped__._telemetry_started` attribute. If it exists, it means the function has been decorated with `@log_call()`, you can use it to verify what's logged:
To unit test decorated functions, call the function and check `__wrapped__._telemetry_success` attribute. If it exists, it means the function has been decorated with `@log_call()`, you can use it to verify what's logged:
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it'd make more sense to keep the started? I saw there are more started then success (probably since it failed in the middle).

@edublancas edublancas merged commit 63f8cc7 into main Oct 25, 2023
34 checks passed
@edublancas
Copy link
Contributor Author

merging this since I discussed with @idomic that removing the started event is acceptable

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.

2 participants