-
Notifications
You must be signed in to change notification settings - Fork 399
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
[FEAT] argilla server
: add user and server id on telemetry metrics
#5445
Conversation
…stem context Also, removed unnecessary methods and attributes.
…y-to-the-argilla-server' into feat/argilla-server/add-user-and-server-id-on-telemetry-metrics
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat/5204-feature-add-huggingface_hubutilssend_telemetry-to-the-argilla-server #5445 +/- ##
==================================================================================================================
- Coverage 91.34% 91.33% -0.02%
==================================================================================================================
Files 140 141 +1
Lines 5767 5791 +24
==================================================================================================================
+ Hits 5268 5289 +21
- Misses 499 502 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Also, review and simplify the track_data method
|
||
_LOGGER = logging.getLogger(__name__) | ||
|
||
|
||
@dataclasses.dataclass | ||
class TelemetryClient: | ||
_server_id: int = uuid.getnode() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The uuid.getnode
returns an integer value. We can use it "as is".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should check that uuid.getnode()
is returning the same value on HF Spaces even after a factory rebuild.
if count is not None: | ||
user_agent["count"] = count | ||
async def track_data(self, topic: str, data: dict): | ||
library_name = "argilla-server" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This aligns the library name (argilla-server) to the package published in pypi.org
|
||
_LOGGER = logging.getLogger(__name__) | ||
|
||
|
||
@dataclasses.dataclass | ||
class TelemetryClient: | ||
_server_id: int = uuid.getnode() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should check that uuid.getnode()
is returning the same value on HF Spaces even after a factory rebuild.
4290b5f
to
e047a04
Compare
Changes introduced storing the server ID allow reuse of the server ID on restart when persistent storage is enabled. We can add also info about persistent storage on server startup. |
942455d
into
feat/5204-feature-add-huggingface_hubutilssend_telemetry-to-the-argilla-server
Description
This PR restores the server_id for telemetry purposes and also add the user.id and user.role when tracking API requests.
Type of change
How Has This Been Tested
Checklist