Implement time-boxed CLI telemetry with opt-out controls#4927
Implement time-boxed CLI telemetry with opt-out controls#4927arturcic wants to merge 19 commits into
Conversation
|
Here is a bit more context: Implement time-boxed, privacy-focused telemetry for the GitVersion CLI. Introduce a telemetry pipeline that collects narrow CLI usage data for a 3-month window following a release. This system includes automated redaction of sensitive paths and credentials while providing multiple opt-out mechanisms via environment variables and CLI flags. Changes What is left is to decide on the backend where all the usage is saved, preferable with some dashboard we can use to see the different usage types. This is a follow up on the refactoring done for v7 where the new POSIX compliant cli was introduced. @asbjornu @gep13 @HHobeck, do you mind to have a look and suggest a backend solution for this? Thank you. |
86aadfa to
7305c88
Compare
asbjornu
left a comment
There was a problem hiding this comment.
Great stuff! Where and how are you (or we) going to monitor the data collected?
| @@ -0,0 +1,163 @@ | |||
| using System.Globalization; | |||
|
|
|||
| namespace GitVersion; | |||
There was a problem hiding this comment.
Should the Telemetry stuff be inside a GitVersion.Telemetry namespace, perhaps?
| public const string MetadataKey = "GitVersionReleaseDate"; | ||
| public const string Format = "yyyy-MM-dd"; | ||
|
|
||
| public static bool TryGetReleaseDate(Assembly assembly, out DateOnly releaseDate) |
There was a problem hiding this comment.
Why isn't the out parameter of type TelemetryReleaseDate? That would simplify the IsWithinWindow method so it can work on the TelemetryReleaseDate instance instead of being static and taking the release date it as a DateOnly argument.
| public static bool TryGetReleaseDate(Assembly assembly, out DateOnly releaseDate) | |
| public static bool TryGetReleaseDate(Assembly assembly, out TelemetryReleaseDate releaseDate) |
| return DateOnly.TryParseExact(value, Format, CultureInfo.InvariantCulture, DateTimeStyles.None, out releaseDate); | ||
| } | ||
|
|
||
| public static bool IsWithinWindow(DateOnly releaseDate, DateOnly utcToday) => |
There was a problem hiding this comment.
As per the previous suggestion, this would be easier to work with rather than having to go through DateOnly for every interaction.
| public static bool IsWithinWindow(DateOnly releaseDate, DateOnly utcToday) => | |
| public bool IsWithinWindow(DateOnly utcToday) => |
| public const string Verbosity = "verbosity"; | ||
| } | ||
|
|
||
| internal static class TelemetryReleaseDate |
There was a problem hiding this comment.
Why not make TelemetryReleaseDate a proper value object rather than just a collection of static methods?
| internal static class TelemetryReleaseDate | |
| internal class TelemetryReleaseDate |
|
|
||
| public void AddFlag(string name) => AddValues(name, ["true"]); | ||
|
|
||
| public void AddValue(string name, string? value, TelemetryValueKind kind = TelemetryValueKind.Plain) |
There was a problem hiding this comment.
If it isn't much work, I'm thinking it would be better to have a one-time immutable mapping of all arguments we want to collect and their kind here inside TelemetryModels.cs such that it's impossible to pass the wrong kind of TelemetryValueKind, and also making the invocation of the AddValue() and AddValues() methods simpler.
| public void AddValue(string name, string? value, TelemetryValueKind kind = TelemetryValueKind.Plain) | |
| public void AddValue(string name, string? value) |
| AddValues(name, [value], kind); | ||
| } | ||
|
|
||
| public void AddValues(string name, IEnumerable<string>? values, TelemetryValueKind kind = TelemetryValueKind.Plain) |
There was a problem hiding this comment.
| public void AddValues(string name, IEnumerable<string>? values, TelemetryValueKind kind = TelemetryValueKind.Plain) | |
| public void AddValues(string name, IEnumerable<string>? values) |
| } | ||
|
|
||
| private void MapParsedValues(Arguments arguments, ParseResult parseResult, CommandOptions options) | ||
| private void MapParsedValues( |
There was a problem hiding this comment.
It would be nice to reduce the complexity of MapParsedValues from 84 as suggested by SonarCloud. Perhaps not in this PR, but it might be relevant for what this PR wants to do to map arguments to TelemetryValueKind. So perhaps something that could be done first to make the implementation of Telemetry simpler? Make the change easy, then make the easy change. :)
That's exactly why I tagged everybody, we need to find a solution for that, and might require some infra to setup for the telemetry. If there is any past experience/ best practices from you, that would be great. |
|
FYI: I have seen that reqnroll provides some reports with statistics as well. Maybe we can use the experience of the project member. Or reuse part of the source code. |
Being the maintainer of Sentry's C# SDK for a while, I have good experience with them. We can apply for an open source (free) license if we want. Sentry is at its core geared more towards error monitoring, but it can be used for analytics as well. And I think it would be useful to also implement its error reporting mechanisms into GitVersion if we choose to go in this direction.
Very interesting @HHobeck! I really like the transparency. This is something we could (mostly) automate with a GitHub action to extract data from the backend we choose and create a post that can be published monthly on gitversion.net. Do you know which analytics backend Reqnroll uses? |
I've being researching and found https://github.com/aptabase/aptabase and https://github.com/aptabase/self-hosting, but then we need to host it ourselves |
|
Please review my proposal I want to ask the maintainer of reqnroll:
|
|
Working with telemetry is not something that I have any experience with, so I am not sure how much, if any, I can offer here. I only thing I would ask for clarification on is, is the default to opt-in to telemetry, or to opt-out? I have seen some backlash recently about decisions made to make the default opt-in for things. |
This looks good to me! |
|
The was the recent change (not linking directly)
|
at least for OSS projects, it's said the user has to opt-out of the telemetry, and by default we will collect the telemetry (at least I've see a couple of cli tools, including As for the opt-in or opt-out, I prefer the user to have to opt-out as I guess the users most probably will not now there is a telemetry they want to opt-in or even if they know, they might not want to. |
thanks for sharing |
same |
|
we could also get inspired by how homebrew does it. What is really important is to be really transparent on what we want to collect and as seen in the brew case, have the analytics be publicaly available |
7305c88 to
9f68dc6
Compare
|
|
I got an answer from Gaspar: |
802d361 to
af891e1
Compare
af891e1 to
da288b6
Compare
f59a718 to
c2dc611
Compare
Research and phased plan for replacing LibGit2Sharp with a vendored managed reader plus git CLI writes (GitTools#5031). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…primitives New project (with NOTICE attribution for code ported from Nerdbank.GitVersioning ManagedGit) and the raw building blocks: hash-agnostic GitObjectId, zlib and random-access streams, text decoding with Latin-1 fallback, and file helpers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Packfile access with idx v2 lookup, ofs/ref delta stream application, a bounded pack memory cache with refcounted views, and multi-pack-index support. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Author/committer signatures with commit-encoding handling, streaming tree reads for diff traversal, and annotated-tag target resolution. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Single lookup facade across loose objects, packs, and the multi-pack index, with SHA-256 repository detection failing fast with a clear message. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Loose and packed refs (peeled entries, symrefs, HEAD, lock-file skipping), walk-up discovery with .git-file indirection, worktree and commondir resolution, shallow detection, git config parsing, and reftable/objectformat rejection with clear messages. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Time/topological sorts with exact tie-break mechanics, first-parent walks, mark-uninteresting limiting, paint-down-to-common merge-base selection, and shallow boundaries grafted parentless from .git/shallow; unexplained missing parents fail the walk. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Changed-paths tree diff aligned by raw byte order, .git/index v2-v4 reader (v4 prefix compression), gitignore matching, and the working-tree status walk matching the libgit2 adapter's uncommitted-changes expression. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Plumbing-only parsing, ArgumentList invocation with LC_ALL=C and terminal prompts disabled, per-invocation auth headers, stderr classification mapped to existing exceptions (LockedFileException retry preserved), and shared pull-request tip matching with peeled-entry folding. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Managed* adapters implement IGitRepository/IMutatingGitRepository and the collection interfaces directly over the managed reader and CLI mutator, with an immutable session snapshot invalidated after mutations. Backend selection is centralized in GitBackendSelector: GITVERSION_GIT_BACKEND picks libgit2 (default when unset) or managed at every composition root, failing fast on unrecognized values. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Deep-equality parity assertions across both backends on adversarial fixtures, plus a real-world corpus script diffing gitversion JSON output across repositories and backends. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds the git_backend dimension so every leg runs with libgit2 and managed, publishing per-backend test summaries and coverage. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GITVERSION_GIT_BACKEND usage, the v7.0 libgit2 default and v7.1 managed flip, and the git-on-PATH requirement scoped to normalization scenarios. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add shared IntelliJ/Rider run configurations to facilitate debugging the CLI using both libgit2sharp and managed git backends.
Implement a telemetry pipeline to collect redacted CLI usage data for OSS design decisions. Users can opt out via environment variables `DO_NOT_TRACK`, `GITVERSION_TELEMETRY_OPTOUT`, or the `--telemetry-opt-out` flag.
Embed release dates in assembly metadata during build and disable telemetry if the window has expired or metadata is missing.
Detect the current CI environment and identify whether the CLI was invoked directly or via GitVersion.MsBuild to better understand usage patterns.
…older Organize the GitVersion.App project structure by grouping telemetry components into their own directory.
…amespaces Remove fully qualified names for build agent interfaces and assembly reflection types to improve code readability.
c2dc611 to
373b37c
Compare
|



Description
Introduces an optional telemetry pipeline to collect CLI usage data, featuring automated redaction of paths and sensitive values. Collection is restricted to a 3-month window following the release date and supports multiple opt-out mechanisms including environment variables and a CLI flag.
Related Issue
Resolves #XYZ
Motivation and Context
This change helps maintainers make data-driven OSS design decisions by understanding common CLI patterns and environment contexts while preserving user privacy through strict redaction and transparency notices.
How Has This Been Tested?
Tested via new unit test suites covering argument parsing, redaction logic, environment-based opt-out, and release window validation.
Screenshots (if appropriate):
Checklist: