Skip to content

Commit 2b95698

Browse files
derrickstoleeclaude
andcommitted
feat: Add important_events capture to filter settings
Context: Operators monitoring Git processes that run gvfs-helper subcommands need visibility into specific data event values (error strings, curl codes, etc.) even when verbose telemetry is disabled. At dl:summary level all data events are currently dropped, so there is no way to guarantee critical diagnostic values surface in the OTEL process span without enabling expensive full-verbose collection site-wide. Justification: The rules live in filter.yml, not summary.yml, because capture is a filtering concern: it determines which data events are guaranteed to appear regardless of detail level, paralleling how filter rules control verbosity. The captured values are stored in their own OTEL attribute (trace2.process.important_events) separate from trace2.process.summary, keeping aggregated metrics distinct from verbatim strings. The capture call in apply__data_generic runs before any early-return paths so that orphaned nested events (nesting > 1 with an empty region stack) are never silently dropped. Implementation: ImportantEventRule (category exact match, key_prefix prefix match, field_name output key) is added to FilterSettings and validated in parseFilterSettingsFromBuffer. TrProcess gains an importantEvents map that is allocated only when rules exist, so the nil check in apply__important_events serves as a fast no-op when the feature is unconfigured. apply__important_events in filter_settings.go matches each data event against the configured rules and appends matching values. emitProcessSpan emits trace2.process.important_events as a JSON object whenever the map is non-empty, at all detail levels. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent cf539cc commit 2b95698

7 files changed

Lines changed: 900 additions & 13 deletions

evt_apply.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -830,6 +830,14 @@ func apply__data_generic(tr2 *trace2Dataset, evt *TrEvent) (err error) {
830830
// nesting level n-1 is stored at regionStack[n-2] (assuming the
831831
// Git process properly sets things up).
832832

833+
// Capture matching data values into importantEvents regardless of
834+
// nesting level. This must run before any early returns below
835+
// so that capture is not skipped when region attachment fails.
836+
apply__important_events(tr2,
837+
evt.pm_generic_data.mf_category,
838+
evt.pm_generic_data.mf_key,
839+
evt.pm_generic_data.mf_generic_value)
840+
833841
if evt.pm_generic_data.mf_nesting <= 1 {
834842
tr2.process.setGenericDataValue(evt.pm_generic_data.mf_category,
835843
evt.pm_generic_data.mf_key, evt.pm_generic_data.mf_generic_value)

filter_settings.go

Lines changed: 67 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,36 @@ import (
1010
// look for in the Trace2 event stream to help us decide how to
1111
// filter data for a particular command.
1212
type FilterSettings struct {
13-
Keynames FilterKeynames `mapstructure:"keynames"`
14-
Nicknames FilterNicknames `mapstructure:"nicknames"`
15-
Rulesets FilterRulesets `mapstructure:"rulesets"`
16-
Defaults FilterDefaults `mapstructure:"defaults"`
13+
Keynames FilterKeynames `mapstructure:"keynames"`
14+
Nicknames FilterNicknames `mapstructure:"nicknames"`
15+
Rulesets FilterRulesets `mapstructure:"rulesets"`
16+
Defaults FilterDefaults `mapstructure:"defaults"`
17+
ImportantEvents []ImportantEventRule `mapstructure:"important_events"`
1718

1819
// The set of custom rulesets defined in YML are each parsed
1920
// and loaded into definitions so that we can use them.
2021
rulesetDefs map[string]*RulesetDefinition
2122
}
2223

24+
// ImportantEventRule defines a rule for promoting values from data events
25+
// that match a specific (category, key prefix) pair into the process
26+
// summary, regardless of the active detail level. This lets operators
27+
// guarantee that certain data event values are always captured and
28+
// surfaced in the OTEL process span even when verbose telemetry is
29+
// disabled. Multiple matching values are collected into an array.
30+
type ImportantEventRule struct {
31+
// Category is the data event category to match (exact match)
32+
Category string `mapstructure:"category"`
33+
34+
// KeyPrefix is the string prefix to match at the beginning of
35+
// the data event's key field
36+
KeyPrefix string `mapstructure:"key_prefix"`
37+
38+
// FieldName is the name of the field in the summary object
39+
// where matched values will be stored (always as an array)
40+
FieldName string `mapstructure:"field_name"`
41+
}
42+
2343
// FilterKeynames defines the names of the Git config settings that
2444
// will be used in `def_param` events to send repository/worktree
2545
// data to us. This lets a site have their own namespace for
@@ -100,9 +120,52 @@ func parseFilterSettingsFromBuffer(data []byte, path string) (*FilterSettings, e
100120
}
101121
}
102122

123+
fieldNames := make(map[string]bool)
124+
for i, rule := range fs.ImportantEvents {
125+
if len(rule.Category) == 0 {
126+
return nil, fmt.Errorf("important_events[%d]: category cannot be empty", i)
127+
}
128+
if len(rule.KeyPrefix) == 0 {
129+
return nil, fmt.Errorf("important_events[%d]: key_prefix cannot be empty", i)
130+
}
131+
if len(rule.FieldName) == 0 {
132+
return nil, fmt.Errorf("important_events[%d]: field_name cannot be empty", i)
133+
}
134+
if fieldNames[rule.FieldName] {
135+
return nil, fmt.Errorf("important_events[%d]: duplicate field_name '%s'", i, rule.FieldName)
136+
}
137+
fieldNames[rule.FieldName] = true
138+
}
139+
103140
return fs, nil
104141
}
105142

143+
// apply__important_events checks if a data event matches any configured
144+
// important_events rules and appends the event's value to the
145+
// importantEvents map if a match is found. Matching events are captured
146+
// regardless of nesting level or detail level.
147+
func apply__important_events(tr2 *trace2Dataset, category string, key string, value interface{}) {
148+
if tr2.process.importantEvents == nil {
149+
return
150+
}
151+
152+
if tr2.rcvr_base == nil || tr2.rcvr_base.RcvrConfig == nil {
153+
return
154+
}
155+
156+
fs := tr2.rcvr_base.RcvrConfig.filterSettings
157+
if fs == nil {
158+
return
159+
}
160+
161+
for _, rule := range fs.ImportantEvents {
162+
if category == rule.Category && strings.HasPrefix(key, rule.KeyPrefix) {
163+
tr2.process.importantEvents[rule.FieldName] = append(
164+
tr2.process.importantEvents[rule.FieldName], value)
165+
}
166+
}
167+
}
168+
106169
// Add a ruleset to the filter settings. This is primarily for writing test code.
107170
func (fs *FilterSettings) addRuleset(rs_name string, path string, rsdef *RulesetDefinition) {
108171
if fs.Rulesets == nil {

filter_settings_test.go

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,81 @@ func x_TryLoadRuleset(t *testing.T, fs *FilterSettings, name string, path string
290290

291291
// //////////////////////////////////////////////////////////////
292292

293+
// //////////////////////////////////////////////////////////////
294+
// important_events validation tests
295+
296+
func Test_ImportantEvents_Valid(t *testing.T) {
297+
yml := `
298+
important_events:
299+
- category: "gvfs-helper"
300+
key_prefix: "error/"
301+
field_name: "gvfs_helper_errors"
302+
- category: "network"
303+
key_prefix: "timeout/"
304+
field_name: "network_timeouts"
305+
`
306+
fs, err := parseFilterSettingsFromBuffer([]byte(yml), "test.yml")
307+
assert.NoError(t, err)
308+
assert.NotNil(t, fs)
309+
assert.Equal(t, 2, len(fs.ImportantEvents))
310+
assert.Equal(t, "gvfs-helper", fs.ImportantEvents[0].Category)
311+
assert.Equal(t, "error/", fs.ImportantEvents[0].KeyPrefix)
312+
assert.Equal(t, "gvfs_helper_errors", fs.ImportantEvents[0].FieldName)
313+
}
314+
315+
func Test_ImportantEvents_EmptyCategory_Rejected(t *testing.T) {
316+
yml := `
317+
important_events:
318+
- category: ""
319+
key_prefix: "error/"
320+
field_name: "errors"
321+
`
322+
_, err := parseFilterSettingsFromBuffer([]byte(yml), "test.yml")
323+
assert.Error(t, err)
324+
assert.Contains(t, err.Error(), "category cannot be empty")
325+
}
326+
327+
func Test_ImportantEvents_EmptyKeyPrefix_Rejected(t *testing.T) {
328+
yml := `
329+
important_events:
330+
- category: "gvfs-helper"
331+
key_prefix: ""
332+
field_name: "errors"
333+
`
334+
_, err := parseFilterSettingsFromBuffer([]byte(yml), "test.yml")
335+
assert.Error(t, err)
336+
assert.Contains(t, err.Error(), "key_prefix cannot be empty")
337+
}
338+
339+
func Test_ImportantEvents_EmptyFieldName_Rejected(t *testing.T) {
340+
yml := `
341+
important_events:
342+
- category: "gvfs-helper"
343+
key_prefix: "error/"
344+
field_name: ""
345+
`
346+
_, err := parseFilterSettingsFromBuffer([]byte(yml), "test.yml")
347+
assert.Error(t, err)
348+
assert.Contains(t, err.Error(), "field_name cannot be empty")
349+
}
350+
351+
func Test_ImportantEvents_DuplicateFieldName_Rejected(t *testing.T) {
352+
yml := `
353+
important_events:
354+
- category: "gvfs-helper"
355+
key_prefix: "error/"
356+
field_name: "shared_name"
357+
- category: "network"
358+
key_prefix: "timeout/"
359+
field_name: "shared_name"
360+
`
361+
_, err := parseFilterSettingsFromBuffer([]byte(yml), "test.yml")
362+
assert.Error(t, err)
363+
assert.Contains(t, err.Error(), "duplicate field_name")
364+
}
365+
366+
// //////////////////////////////////////////////////////////////
367+
293368
func Test_Nil_Nil_FilterSettings(t *testing.T) {
294369

295370
dl, dl_debug := computeDetailLevel(nil, nil, x_qn)

0 commit comments

Comments
 (0)