feat: persist a stable machine ID for telemetry [on hold: add-on vs Client ownership]

[PetrDlouhy]

On hold — design question open: should the machine ID be owned by the add-on (as here) or by the Client? Split out of the telemetry work so that can ship meanwhile. Now contains only the ID change.

Problem

system_id came from uuid.getnode() — the MAC — in both Python and the Client. It churns with MAC randomization, virtual interfaces and Go/Python's own random fallback, so one machine looks like many: new-machine counts inflate and every machine-level rate is biased, most importantly the sign-in-rate denominator that carries the growth diagnosis.

What this does

paths.get_stable_system_id() stores a 15-digit ID in the global data directory and reuses it. Two deliberate choices:

• First run stores the current uuid.getnode(), so machines keep the ID they already report — no discontinuity in existing series; only future churn stops.
• The file is deliberately dumb (bare 15 digits, no fingerprinting). Deleting blenderkit_data or corrupting the file just resets to getnode(), which lands on the same ID whenever the MAC is stable. A copied file (VM image, synced home dir) makes two machines share an ID — measured at ~0.02% on production (5 of 26,917 machines/day serve 5+ accounts, none 20+), and the obvious guard (hostname marker) would re-introduce churn on macOS where hostnames change with the network. Trade-off documented in the docstring rather than defended with code.

Blendkit-Client reads the same file (BlenderKit/bk_client#26) and the add-on passes it via --system_id when it spawns one, so a standalone Client — or one started by another software's add-on — still reports the same machine.

Open question for review

Ownership. Arguments for the add-on: it knows global_dir/XDG semantics and is the only writer. For the Client: it's the long-lived, cross-software process and already stamps every request header, so the ID would live where it's used. Happy to move it.

Expected effect on metrics

After rollout, machine-churn inflation stops: new-machine counts deflate slightly and the sign-in rate ticks up mechanically. Dashboards should be annotated at release so it isn't read as a behaviour change.

Stack

#2227 → #2238 (login telemetry) → this

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 30.52%. Comparing base (2f5e935) to head (4f2c819).

Additional details and impacted files

@@                        Coverage Diff                         @@
##           feature/login-funnel-telemetry    #2228      +/-   ##
==================================================================
+ Coverage                           30.32%   30.52%   +0.20%     
==================================================================
  Files                                  79       79              
  Lines                               25816    25892      +76     
==================================================================
+ Hits                                 7828     7904      +76     
  Misses                              17988    17988              

Flag	Coverage Δ
python	30.52% <100.00%> (+0.20%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
bkit_oauth.py	59.23% <100.00%> (-0.26%)	⬇️
client_lib.py	59.34% <100.00%> (+0.28%)	⬆️
paths.py	55.32% <100.00%> (+3.28%)	⬆️
tests/test_bkit_oauth.py	100.00% <100.00%> (ø)
tests/test_paths.py	99.60% <100.00%> (+0.08%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Tweekazoid]

vse funguje

[agajdosi]

🥇

[agajdosi]

Stable until you remove the data directory.
Stable until you tweak the file.

[PetrDlouhy]

Both true, and the second one has a sharper edge than a hand edit: the file lives in the home directory, so VM images, cloned lab installs and synced/roaming home dirs carry it to another machine — many machines would then report one ID, undercounting machines exactly as badly as MAC churn overcounts them.

Fixed in cf50399:

• A hostname marker is stored next to the ID (sha256-truncated — hostnames often contain personal names and only equality matters here). On mismatch the ID is regenerated from that machine's uuid.getnode() instead of reusing a travelled one. MAC randomization doesn't touch the hostname, so the case this helper exists for still keeps its ID.
• Deleting blenderkit_data, editing the file, or renaming the machine all just reset to getnode() — so a machine with a stable MAC lands on the same ID anyway (no churn); only one whose MAC drifted since gets a new one.
• The docstring now spells these limits out instead of claiming plain "stable".

New tests cover the copied-file case, a malformed id inside valid JSON, and that the marker is hashed rather than the raw hostname.

[PetrDlouhy]

Correction to my previous reply — I checked the numbers and reverted the hostname marker (088d178).

I sized the risk it defended against on production: on a sample day, 5 of 26,917 machines served 5+ accounts, and none served 20+. So "one file copied to many machines" is ~0.02% — while the marker costs stability in exactly the population this feature exists for, since macOS hostnames change with the network (.local / mDNS renames) and every flap would regenerate the ID. Bad trade; the file is a bare 15-digit value again, with the limits you pointed out written into the docstring rather than defended with fingerprinting.

Your comment did surface a second, real problem though. The add-on passes --system_id only to a Client it spawns. A standalone Client (tray app) — or one started by another software's add-on — got no flag and kept using the MAC-derived ID, while the add-on used the persisted one: the same machine reporting two IDs, breaking the machine↔login join precisely where the Client is shared. Fixed in bk_client (bb727ff): precedence is now --system_id > persisted file > MAC, with the Client only ever reading the file and the add-on remaining its sole writer. Verified end-to-end that the Python writer and the Go reader agree on the same path and value (incl. XDG_DATA_HOME).

[PetrDlouhy]

Rebuilt as a system_id-only PR, stacked on #2238 (login telemetry), so the telemetry work isn't blocked by the ownership question you raised.

Also worth knowing: bk_client#24 already merged yesterday, so the Client side already has --system_id and the /report_event route in main. The only Client piece still open is the standalone-Client file read — re-cut cleanly on current main as BlenderKit/bk_client#28, also marked on hold. (My earlier telemetry-only Client PR #27 was redundant and is closed.)

Submodule pointers now match that: #2238 → main (v1.12.2, has the route); this PR → bk_client#28.