Skip to content

Commit 3637d1c

Browse files
committed
add privateMetadata fields
1 parent 1574f38 commit 3637d1c

2 files changed

Lines changed: 32 additions & 19 deletions

File tree

‎internal/telemetry/telemetry.go‎

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -67,23 +67,24 @@ func NewRecorder(client api.Client, logger log.Logger, clientVersion string) *re
6767

6868
// Record sends an event on a best-effort basis. Feature and action identify the
6969
// event (for example, "srcCli.search" and "succeeded"). Metadata must contain
70-
// only numeric, PII-free facts. Network, GraphQL, timeout, and old-instance
71-
// failures are logged at debug level. Record applies its own timeout, so the
72-
// caller's context need not carry a deadline.
73-
func (r *recorder) Record(ctx context.Context, feature, action string, metadata map[string]float64) {
74-
if err := r.record(ctx, feature, action, metadata); err != nil {
70+
// only numeric, PII-free facts. Private metadata may contain arbitrary JSON and
71+
// is not exported from Sourcegraph instances by default. Network, GraphQL,
72+
// timeout, and old-instance failures are logged at debug level. Record applies
73+
// its own timeout, so the caller's context need not carry a deadline.
74+
func (r *recorder) Record(ctx context.Context, feature, action string, metadata map[string]float64, privateMetadata map[string]any) {
75+
if err := r.record(ctx, feature, action, metadata, privateMetadata); err != nil {
7576
r.logger.Debug("recording telemetry event", log.String("feature", feature), log.String("action", action), log.Error(err))
7677
}
7778
}
7879

7980
// record does the work behind Record and returns any error, so it can be tested
8081
// directly. Callers outside tests should use Record.
81-
func (r *recorder) record(ctx context.Context, feature, action string, metadata map[string]float64) error {
82+
func (r *recorder) record(ctx context.Context, feature, action string, metadata map[string]float64, privateMetadata map[string]any) error {
8283
ctx, cancel := context.WithTimeout(ctx, r.timeout)
8384
defer cancel()
8485

8586
vars := map[string]any{
86-
"events": []any{buildEventInput(r.clientVersion, feature, action, metadata)},
87+
"events": []any{buildEventInput(r.clientVersion, feature, action, metadata, privateMetadata)},
8788
}
8889

8990
payload, err := json.Marshal(map[string]any{
@@ -121,18 +122,22 @@ func (r *recorder) record(ctx context.Context, feature, action string, metadata
121122
}
122123

123124
// buildEventInput builds a single TelemetryEventInput as a JSON-serializable map.
124-
func buildEventInput(clientVersion, feature, action string, metadata map[string]float64) map[string]any {
125+
func buildEventInput(clientVersion, feature, action string, metadata map[string]float64, privateMetadata map[string]any) map[string]any {
126+
parameters := map[string]any{
127+
"version": eventParametersVersion,
128+
"metadata": buildMetadata(metadata),
129+
}
130+
if privateMetadata != nil {
131+
parameters["privateMetadata"] = privateMetadata
132+
}
125133
return map[string]any{
126134
"feature": feature,
127135
"action": action,
128136
"source": map[string]string{
129137
"client": clientName,
130138
"clientVersion": clientVersion,
131139
},
132-
"parameters": map[string]any{
133-
"version": eventParametersVersion,
134-
"metadata": buildMetadata(metadata),
135-
},
140+
"parameters": parameters,
136141
}
137142
}
138143

‎internal/telemetry/telemetry_test.go‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,9 @@ func TestRecord_SendsWellFormedMutation(t *testing.T) {
7777
rec.Record(context.Background(), "srcCli.search", "succeeded", map[string]float64{
7878
"durationMs": 12,
7979
"exitCode": 0,
80+
}, map[string]any{
81+
"queryType": "literal",
82+
"streamed": true,
8083
})
8184

8285
assert.Equal(t, recordEventsMutation, gotPayload.Query)
@@ -99,6 +102,10 @@ func TestRecord_SendsWellFormedMutation(t *testing.T) {
99102
map[string]any{"key": "durationMs", "value": float64(12)},
100103
map[string]any{"key": "exitCode", "value": float64(0)},
101104
}, parameters["metadata"])
105+
assert.Equal(t, map[string]any{
106+
"queryType": "literal",
107+
"streamed": true,
108+
}, parameters["privateMetadata"])
102109

103110
client.AssertExpectations(t)
104111
}
@@ -121,12 +128,13 @@ func TestRecord_NilMetadataSendsEmptyList(t *testing.T) {
121128

122129
logger := log.NoOp()
123130
rec := NewRecorder(client, logger, testClientVersion)
124-
rec.Record(context.Background(), "srcCli.version", "succeeded", nil)
131+
rec.Record(context.Background(), "srcCli.version", "succeeded", nil, nil)
125132

126133
event := gotPayload.Variables["events"].([]any)[0].(map[string]any)
127134
parameters := event["parameters"].(map[string]any)
128135
assert.Equal(t, float64(eventParametersVersion), parameters["version"])
129136
assert.Equal(t, []any{}, parameters["metadata"])
137+
assert.NotContains(t, parameters, "privateMetadata")
130138
}
131139

132140
func TestRecord_NetworkErrorSwallowed(t *testing.T) {
@@ -140,7 +148,7 @@ func TestRecord_NetworkErrorSwallowed(t *testing.T) {
140148

141149
// Must not panic and must not surface the error.
142150
assert.NotPanics(t, func() {
143-
rec.Record(context.Background(), "srcCli.search", "failed", nil)
151+
rec.Record(context.Background(), "srcCli.search", "failed", nil, nil)
144152
})
145153
logs := exportLogs()
146154
if assert.Len(t, logs, 1) {
@@ -150,7 +158,7 @@ func TestRecord_NetworkErrorSwallowed(t *testing.T) {
150158
}
151159

152160
// record itself reports the error for callers that want it.
153-
err := rec.record(context.Background(), "srcCli.search", "failed", nil)
161+
err := rec.record(context.Background(), "srcCli.search", "failed", nil, nil)
154162
assert.Error(t, err)
155163
}
156164

@@ -165,7 +173,7 @@ func TestRecord_GraphQLErrorSwallowed(t *testing.T) {
165173
logger := log.NoOp()
166174
rec := NewRecorder(client, logger, testClientVersion)
167175
assert.NotPanics(t, func() {
168-
rec.Record(context.Background(), "srcCli.search", "succeeded", nil)
176+
rec.Record(context.Background(), "srcCli.search", "succeeded", nil, nil)
169177
})
170178
}
171179

@@ -203,7 +211,7 @@ func TestRecord_OAuthUnauthorizedDoesNotWriteToStdout(t *testing.T) {
203211

204212
logger := log.NoOp()
205213
rec := NewRecorder(client, logger, testClientVersion)
206-
rec.Record(context.Background(), "srcCli.search", "succeeded", nil)
214+
rec.Record(context.Background(), "srcCli.search", "succeeded", nil, nil)
207215

208216
if err := stdoutWriter.Close(); err != nil {
209217
t.Fatal(err)
@@ -237,7 +245,7 @@ func TestRecord_AppliesTimeout(t *testing.T) {
237245
logger := log.NoOp()
238246
rec := NewRecorder(client, logger, testClientVersion)
239247
rec.timeout = 50 * time.Millisecond
240-
rec.Record(context.Background(), "srcCli.search", "succeeded", nil)
248+
rec.Record(context.Background(), "srcCli.search", "succeeded", nil, nil)
241249

242250
assert.True(t, hadDeadline, "expected Record to apply a context deadline")
243251
}
@@ -253,7 +261,7 @@ func TestRecord_TimeoutCancelsHTTPRequest(t *testing.T) {
253261
defer cancel()
254262
done := make(chan struct{})
255263
go func() {
256-
rec.Record(ctx, "srcCli.search", "succeeded", nil)
264+
rec.Record(ctx, "srcCli.search", "succeeded", nil, nil)
257265
close(done)
258266
}()
259267

0 commit comments

Comments
 (0)