feat: telemetry, error tracking, CLI & config manager #538
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Conditions required to send errors (ALL conditions must be met or no
errors will be reported):
- sentry_sdk package is installed (Maybe we make it a dependency?)
- sync=True in settings
- pytest is not running
- running in a pip package installation
- running in a non-git directory
- online environment
- CLI config commands are needed to allow the user to be able to edit
them, but maybe it'll be too large for this PR.
TODOs:
- [x] remove verbose sentry log msg (next PR)
- [x] needs lancedb.__version__ to tag the version of the error origin
---------
Co-authored-by: Lance Release <lance-dev@lancedb.com>
Co-authored-by: Rob Meng <rob.xu.meng@gmail.com>
Co-authored-by: Will Jones <willjones127@gmail.com>
Co-authored-by: Chang She <759245+changhiskhan@users.noreply.github.com>
Co-authored-by: rmeng <rob@lancedb.com>
Co-authored-by: Chang She <chang@lancedb.com>
Co-authored-by: Rok Mihevc <rok@mihevc.org>
Depends on - #492 Current approach: * All events are batched and sent once every time the `rate_limit` is crossed. All events past `max_events` limit are dropped within each `rate_limit` timeframe. This means that we're not capturing the exact usage and we'll need to compare relative usage * Currently the `rate_limit` is set to 60seconds, meaning there will be 1 request made after each 60 seconds. `max_events` is set to 25 which means maximum 25 events will be captures at a time and will be dropped past that. These numbers will need to be tuned according to our needs. Introduced Events class to track events without disrupting any workflow. Allows setting rate limits. EDIT: - [ ] Ohh need to turn on sentry integration too --------- Co-authored-by: Lance Release <lance-dev@lancedb.com> Co-authored-by: Rob Meng <rob.xu.meng@gmail.com> Co-authored-by: Will Jones <willjones127@gmail.com> Co-authored-by: Chang She <759245+changhiskhan@users.noreply.github.com> Co-authored-by: rmeng <rob@lancedb.com> Co-authored-by: Chang She <chang@lancedb.com> Co-authored-by: Rok Mihevc <rok@mihevc.org>
Usage: `lancedb` `lancedb --help` `lancedb diagnostics --enabled` , `lancedb disgnostics --disabled` `lancedb config`
AyushExel
commented
Oct 5, 2023
AyushExel
commented
Oct 5, 2023
AyushExel
commented
Oct 5, 2023
AyushExel
commented
Oct 5, 2023
changhiskhan
approved these changes
Oct 6, 2023
There was a problem hiding this comment.
just realized there are no tests for these.
could you add tests when you get a chance?
for click: https://click.palletsprojects.com/en/8.1.x/testing/
for sentry/posthog: use pytest-mock
you'll need to have some option to turn on diagnostics during CI but it should only send to the mock endpoint and then you can assert the event attributes is what you expect.
AyushExel
commented
Oct 6, 2023
python/tests/test_telemetry.py
Outdated
| # TODO: don't hardcode these here. Instead create a module level json scehma in lancedb.utils.events for better evolvability | ||
| batch_keys = ["api_key", "distinct_id", "batch"] | ||
| event_keys = ["event", "properties", "timestamp", "distinct_id"] | ||
| property_keys = ["cli", "install", "platforms", "version", "session_id", "blud"] |
There was a problem hiding this comment.
Added loose tests for now here. Ideally we could hardcode the json schema as global var and directly assert here but I don't want to edit the reviewed files so will do in another PR.
And sentry can't be tested the same way as it's integrated via the sdk and there are no manual API calls. Nothing to worry there though, the whole sentry workflow is wrapped in exception handler (https://github.com/lancedb/lancedb/pull/538/files#diff-603e1842b90966a1a2eb9e41f61c62a6db0fd6b3d5a71cec2f0355387c3d7eb7R33)
|
Wanted to use responses for requests testing but I don't think adding dependency for a single test would be the best idea so just put together something. Take a look and merge if all looks good. |
raghavdixit99
pushed a commit
to raghavdixit99/lancedb
that referenced
this pull request
Apr 5, 2024
Co-authored-by: Lance Release <lance-dev@lancedb.com> Co-authored-by: Rob Meng <rob.xu.meng@gmail.com> Co-authored-by: Will Jones <willjones127@gmail.com> Co-authored-by: Chang She <759245+changhiskhan@users.noreply.github.com> Co-authored-by: rmeng <rob@lancedb.com> Co-authored-by: Chang She <chang@lancedb.com> Co-authored-by: Rok Mihevc <rok@mihevc.org>
alexkohler
pushed a commit
to alexkohler/lancedb
that referenced
this pull request
Apr 20, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.