diff --git a/matcher/ruleset_test.go b/matcher/ruleset_test.go index 6f76972..2274dcd 100644 --- a/matcher/ruleset_test.go +++ b/matcher/ruleset_test.go @@ -170,6 +170,7 @@ func (r mockRuleSet) Version() int { return r.version func (r mockRuleSet) CutoverNanos() int64 { return r.cutoverNanos } func (r mockRuleSet) Tombstoned() bool { return r.tombstoned } func (r mockRuleSet) ActiveSet(timeNanos int64) rules.Matcher { return r.matcher } +func (r mockRuleSet) ToMutableRuleSet() rules.MutableRuleSet { return nil } func testRuleSet() (kv.Store, Cache, *ruleSet, Options) { store := mem.NewStore() diff --git a/rules/mapping.go b/rules/mapping.go index c7a0a8c..61ebe74 100644 --- a/rules/mapping.go +++ b/rules/mapping.go @@ -62,14 +62,15 @@ func newMappingRuleSnapshot( if err != nil { return nil, err } - return &mappingRuleSnapshot{ - name: r.Name, - tombstoned: r.Tombstoned, - cutoverNanos: r.CutoverTime, - filter: filter, - policies: policies, - rawFilters: r.TagFilters, - }, nil + + return newMappingRuleSnapshotFromFields( + r.Name, + r.Tombstoned, + r.CutoverTime, + r.TagFilters, + policies, + filter, + ), nil } func newMappingRuleSnapshotFromFields( @@ -78,13 +79,15 @@ func newMappingRuleSnapshotFromFields( cutoverNanos int64, tagFilters map[string]string, policies []policy.Policy, -) mappingRuleSnapshot { - return mappingRuleSnapshot{ + filter filters.Filter, +) *mappingRuleSnapshot { + return &mappingRuleSnapshot{ name: name, tombstoned: tombstoned, cutoverNanos: cutoverNanos, policies: policies, rawFilters: tagFilters, + filter: filter, } } @@ -106,13 +109,14 @@ func newMappingRuleSnapshotJSON(mrs mappingRuleSnapshot) mappingRuleSnapshotJSON } } -func (mrsj mappingRuleSnapshotJSON) mappingRuleSnapshot() mappingRuleSnapshot { +func (mrsj mappingRuleSnapshotJSON) mappingRuleSnapshot() *mappingRuleSnapshot { return newMappingRuleSnapshotFromFields( mrsj.Name, mrsj.Tombstoned, mrsj.CutoverNanos, mrsj.TagFilters, mrsj.Policies, + nil, ) } @@ -199,6 +203,7 @@ func (mc *mappingRule) addSnapshot( rawFilters map[string]string, policies []policy.Policy, cutoverTime int64, + ) error { snapshot := newMappingRuleSnapshotFromFields( name, @@ -206,9 +211,10 @@ func (mc *mappingRule) addSnapshot( cutoverTime, rawFilters, policies, + nil, ) - mc.snapshots = append(mc.snapshots, &snapshot) + mc.snapshots = append(mc.snapshots, snapshot) return nil } @@ -295,8 +301,7 @@ func newMappingRuleJSON(mc mappingRule) mappingRuleJSON { func (mrj mappingRuleJSON) mappingRule() mappingRule { snapshots := make([]*mappingRuleSnapshot, len(mrj.Snapshots)) for i, s := range mrj.Snapshots { - newSnapshot := s.mappingRuleSnapshot() - snapshots[i] = &newSnapshot + snapshots[i] = s.mappingRuleSnapshot() } return mappingRule{ uuid: mrj.UUID, diff --git a/rules/namespace.go b/rules/namespace.go index 47b0969..9355209 100644 --- a/rules/namespace.go +++ b/rules/namespace.go @@ -240,9 +240,9 @@ func NewNamespaces(version int, namespaces *schema.Namespaces) (Namespaces, erro }, nil } -// Clone creates a deep copy of this namespace +// Clone creates a deep copy of this namespace. func (nss Namespaces) Clone() (Namespaces, error) { - // TODO(dgromov): Do an actual deep copy that doesn't rely on .Schema() + // TODO(dgromov): Do an actual deep copy that doesn't rely on .Schema(). schema, err := nss.Schema() if err != nil { return emptyNamespaces, err diff --git a/rules/rollup.go b/rules/rollup.go index c17281f..b198320 100644 --- a/rules/rollup.go +++ b/rules/rollup.go @@ -68,7 +68,7 @@ func newRollupTarget(target *schema.RollupTarget) (RollupTarget, error) { }, nil } -// NewRollupTargetFromFields creates a new rollupTarget from a list of it's component fields. +// NewRollupTargetFromFields creates a new rollupTarget from a list of its component fields. func NewRollupTargetFromFields(name string, tags []string, policies []policy.Policy) RollupTarget { return RollupTarget{Name: []byte(name), Tags: bytesArrayFromStringArray(tags), Policies: policies} } @@ -139,7 +139,7 @@ func newRollupRuleSnapshotJSON(rrs rollupRuleSnapshot) rollupRuleSnapshotJSON { } } -func (rrsj rollupRuleSnapshotJSON) rollupRuleSnapshot() rollupRuleSnapshot { +func (rrsj rollupRuleSnapshotJSON) rollupRuleSnapshot() *rollupRuleSnapshot { targets := make([]RollupTarget, len(rrsj.Targets)) for i, t := range rrsj.Targets { targets[i] = t.rollupTarget() @@ -151,6 +151,7 @@ func (rrsj rollupRuleSnapshotJSON) rollupRuleSnapshot() rollupRuleSnapshot { rrsj.CutoverNanos, rrsj.TagFilters, targets, + nil, ) } @@ -204,14 +205,15 @@ func newRollupRuleSnapshot( if err != nil { return nil, err } - return &rollupRuleSnapshot{ - name: r.Name, - tombstoned: r.Tombstoned, - cutoverNanos: r.CutoverTime, - filter: filter, - targets: targets, - rawFilters: r.TagFilters, - }, nil + + return newRollupRuleSnapshotFromFields( + r.Name, + r.Tombstoned, + r.CutoverTime, + r.TagFilters, + targets, + filter, + ), nil } func newRollupRuleSnapshotFromFields( @@ -220,13 +222,15 @@ func newRollupRuleSnapshotFromFields( cutoverNanos int64, tagFilters map[string]string, targets []RollupTarget, -) rollupRuleSnapshot { - return rollupRuleSnapshot{ + filter filters.Filter, +) *rollupRuleSnapshot { + return &rollupRuleSnapshot{ name: name, tombstoned: tombstoned, cutoverNanos: cutoverNanos, targets: targets, rawFilters: tagFilters, + filter: filter, } } @@ -349,9 +353,10 @@ func (rc *rollupRule) addSnapshot( cutoverTime, rawFilters, rollupTargets, + nil, ) - rc.snapshots = append(rc.snapshots, &snapshot) + rc.snapshots = append(rc.snapshots, snapshot) return nil } @@ -411,8 +416,7 @@ func newRollupRuleJSON(rc rollupRule) rollupRuleJSON { func (rrj rollupRuleJSON) rollupRule() rollupRule { snapshots := make([]*rollupRuleSnapshot, len(rrj.Snapshots)) for i, s := range rrj.Snapshots { - newSnapshot := s.rollupRuleSnapshot() - snapshots[i] = &newSnapshot + snapshots[i] = s.rollupRuleSnapshot() } return rollupRule{ uuid: rrj.UUID, diff --git a/rules/ruleset.go b/rules/ruleset.go index a8a00da..846aabc 100644 --- a/rules/ruleset.go +++ b/rules/ruleset.go @@ -454,6 +454,9 @@ type RuleSet interface { // ActiveSet returns the active ruleset at a given time. ActiveSet(timeNanos int64) Matcher + + // ToMutableRuleSet returns a mutable version of this ruleset. + ToMutableRuleSet() MutableRuleSet } type ruleSet struct { @@ -471,7 +474,8 @@ type ruleSet struct { isRollupIDFn metricID.MatchIDFn } -func newRuleSetFromSchema(version int, rs *schema.RuleSet, opts Options) (*ruleSet, error) { +// NewRuleSetFromSchema creates a new RuleSet from a schema object. +func NewRuleSetFromSchema(version int, rs *schema.RuleSet, opts Options) (RuleSet, error) { if rs == nil { return nil, errNilRuleSetSchema } @@ -508,17 +512,6 @@ func newRuleSetFromSchema(version int, rs *schema.RuleSet, opts Options) (*ruleS }, nil } -// NewRuleSetFromSchema creates a new RuleSet from a schema object. -func NewRuleSetFromSchema(version int, rs *schema.RuleSet, opts Options) (RuleSet, error) { - return newRuleSetFromSchema(version, rs, opts) -} - -// NewMutableRuleSetFromSchema creates a new MutableRuleSet from a schema object. -func NewMutableRuleSetFromSchema(version int, rs *schema.RuleSet) (MutableRuleSet, error) { - // Takes a blank Options stuct because none of the mutation functions need the options. - return newRuleSetFromSchema(version, rs, NewOptions()) -} - // NewEmptyRuleSet returns an empty ruleset to be used with a new namespace. func NewEmptyRuleSet(namespaceName string, meta UpdateMetadata) MutableRuleSet { return &ruleSet{ @@ -560,6 +553,10 @@ func (rs *ruleSet) ActiveSet(timeNanos int64) Matcher { ) } +func (rs *ruleSet) ToMutableRuleSet() MutableRuleSet { + return MutableRuleSet(rs) +} + // resolvePolicies resolves the conflicts among policies if any, following the rules below: // * If two policies have the same resolution but different retention, the one with longer // retention period is chosen. @@ -689,7 +686,7 @@ type MutableRuleSet interface { // Schema returns the schema.Ruleset representation of this ruleset. Schema() (*schema.RuleSet, error) - // Clone returns a copy of this MutableRuleSet + // Clone returns a copy of this MutableRuleSet. Clone() (MutableRuleSet, error) // MarshalJSON serializes this RuleSet into JSON. @@ -758,17 +755,18 @@ func (rs ruleSet) Schema() (*schema.RuleSet, error) { } func (rs ruleSet) Clone() (MutableRuleSet, error) { - // TODO(dgromov): Do an actual deep copy that doesn't rely on .Schema() + // TODO(dgromov): Do an actual deep copy that doesn't rely on .Schema(). schema, err := rs.Schema() if err != nil { return nil, err } - newRuleSet, err := NewMutableRuleSetFromSchema(rs.version, schema) + newRuleSet, err := NewRuleSetFromSchema(rs.version, schema, NewOptions()) if err != nil { return nil, err } - return newRuleSet, nil + + return newRuleSet.ToMutableRuleSet(), nil } // MarshalJSON returns the JSON encoding of staged policies. @@ -1147,8 +1145,8 @@ func (rs ruleSet) getRollupRuleByID(id string) (*rollupRule, error) { // RuleConflictError is returned when a rule modification is made that would conflict with the current state. type RuleConflictError struct { - ConflictUUID string - msg string + ConflictRuleUUID string + msg string } func (e RuleConflictError) Error() string { return e.msg } @@ -1161,7 +1159,7 @@ func (rs ruleSet) validateMappingRuleUpdate(mrd MappingRuleData) error { if n, err := m.Name(); err != nil { continue } else if n == mrd.Name { - return RuleConflictError{msg: fmt.Sprintf("Rule with name: %s already exists", n), ConflictUUID: m.uuid} + return RuleConflictError{msg: fmt.Sprintf("Rule with name: %s already exists", n), ConflictRuleUUID: m.uuid} } } @@ -1177,7 +1175,7 @@ func (rs ruleSet) validateRollupRuleUpdate(rrd RollupRuleData) error { if n, err := r.Name(); err != nil { continue } else if n == rrd.Name { - return RuleConflictError{msg: fmt.Sprintf("Rule with name: %s already exists", n), ConflictUUID: r.uuid} + return RuleConflictError{msg: fmt.Sprintf("Rule with name: %s already exists", n), ConflictRuleUUID: r.uuid} } if len(r.snapshots) == 0 { @@ -1187,7 +1185,7 @@ func (rs ruleSet) validateRollupRuleUpdate(rrd RollupRuleData) error { for _, t1 := range latestSnapshot.targets { for _, t2 := range rrd.Targets { if t1.sameTransform(t2) { - return RuleConflictError{msg: fmt.Sprintf("Same rollup transformation: %s: %v already exists", t1.Name, t1.Tags), ConflictUUID: r.uuid} + return RuleConflictError{msg: fmt.Sprintf("Same rollup transformation: %s: %v already exists", t1.Name, t1.Tags), ConflictRuleUUID: r.uuid} } } } diff --git a/rules/ruleset_test.go b/rules/ruleset_test.go index 6acce64..2670a40 100644 --- a/rules/ruleset_test.go +++ b/rules/ruleset_test.go @@ -646,7 +646,7 @@ func TestRuleSetMarshalJSON(t *testing.T) { func TestRuleSetUnmarshalJSON(t *testing.T) { version := 1 - newRuleSet, err := NewMutableRuleSetFromSchema(version, marshalTestSchema) + newRuleSet, err := newMutableRuleSetFromSchema(version, marshalTestSchema) require.NoError(t, err) data, err := json.Marshal(newRuleSet) @@ -678,7 +678,7 @@ func TestRuleSetSchema(t *testing.T) { RollupRules: testRollupRulesConfig(), } - rs, err := NewMutableRuleSetFromSchema(version, expectedRs) + rs, err := newMutableRuleSetFromSchema(version, expectedRs) require.NoError(t, err) res, err := rs.Schema() require.NoError(t, err) @@ -2199,11 +2199,22 @@ func initMutableTest() (MutableRuleSet, *ruleSet, RuleSetUpdateHelper, error) { RollupRules: testRollupRulesConfig(), } - mutable, err := NewMutableRuleSetFromSchema(version, expectedRs) + mutable, err := newMutableRuleSetFromSchema(version, expectedRs) rs := mutable.(*ruleSet) return mutable, rs, NewRuleSetUpdateHelper(10), err } +// NewMutableRuleSetFromSchema creates a new MutableRuleSet from a schema object. +func newMutableRuleSetFromSchema(version int, rs *schema.RuleSet) (MutableRuleSet, error) { + // Takes a blank Options stuct because none of the mutation functions need the options. + roRuleSet, err := NewRuleSetFromSchema(version, rs, NewOptions()) + if err != nil { + return nil, err + } + rawRs := roRuleSet.(*ruleSet) + return MutableRuleSet(rawRs), nil +} + func TestAddMappingRule(t *testing.T) { mutable, rs, helper, err := initMutableTest() require.NoError(t, err) diff --git a/rules/store.go b/rules/store.go index 1bb86ea..3df222f 100644 --- a/rules/store.go +++ b/rules/store.go @@ -45,7 +45,7 @@ type Store interface { ReadNamespaces() (*Namespaces, error) // ReadRuleSet returns the version and the persisted ruleset data in kv store. - ReadRuleSet(namespaceName string) (MutableRuleSet, error) + ReadRuleSet(namespaceName string) (RuleSet, error) } // StoreOptions is a configuration for the rules r/w store. @@ -88,7 +88,7 @@ func (s store) ReadNamespaces() (*Namespaces, error) { return &nss, err } -func (s store) ReadRuleSet(nsName string) (MutableRuleSet, error) { +func (s store) ReadRuleSet(nsName string) (RuleSet, error) { ruleSetKey := s.ruleSetKey(nsName) value, err := s.kvStore.Get(ruleSetKey) @@ -101,7 +101,8 @@ func (s store) ReadRuleSet(nsName string) (MutableRuleSet, error) { if err := value.Unmarshal(&ruleSet); err != nil { return nil, fmt.Errorf("Could not fetch RuleSet %s: %v", nsName, err.Error()) } - rs, err := NewMutableRuleSetFromSchema(version, &ruleSet) + + rs, err := NewRuleSetFromSchema(version, &ruleSet, NewOptions()) if err != nil { return nil, fmt.Errorf("Could not fetch RuleSet %s: %v", nsName, err.Error()) } diff --git a/rules/store_test.go b/rules/store_test.go index 7d994c4..02eaf23 100644 --- a/rules/store_test.go +++ b/rules/store_test.go @@ -428,17 +428,18 @@ func TestWrite(t *testing.T) { require.Error(t, err) require.Nil(t, nss) - ruleSet, err := NewMutableRuleSetFromSchema(0, testRuleSet) + mutable, err := newMutableRuleSetFromSchema(0, testRuleSet) require.NoError(t, err) + namespaces, err := NewNamespaces(0, testNamespaces) require.NoError(t, err) - err = s.WriteAll(&namespaces, ruleSet) + err = s.WriteAll(&namespaces, mutable) require.NoError(t, err) rs, err = s.ReadRuleSet(testNamespace) require.NoError(t, err) - rsSchema, err := rs.Schema() + rsSchema, err := rs.ToMutableRuleSet().Schema() require.NoError(t, err) require.Equal(t, rsSchema, testRuleSet) @@ -460,7 +461,7 @@ func TestWriteErrorAll(t *testing.T) { require.Error(t, err) require.Nil(t, nss) - ruleSet, err := NewMutableRuleSetFromSchema(1, testRuleSet) + mutable, err := newMutableRuleSetFromSchema(1, testRuleSet) require.NoError(t, err) namespaces, err := NewNamespaces(0, testNamespaces) @@ -476,9 +477,9 @@ func TestWriteErrorAll(t *testing.T) { badPairs := []dataPair{ dataPair{nil, nil}, - dataPair{nil, ruleSet}, + dataPair{nil, mutable}, dataPair{&namespaces, nil}, - dataPair{&otherNss, ruleSet}, + dataPair{&otherNss, mutable}, } for _, p := range badPairs { @@ -504,10 +505,10 @@ func TestWriteErrorRuleSet(t *testing.T) { require.Error(t, err) require.Nil(t, nss) - ruleSet, err := NewMutableRuleSetFromSchema(1, testRuleSet) + mutable, err := newMutableRuleSetFromSchema(1, testRuleSet) require.NoError(t, err) - badRuleSets := []MutableRuleSet{ruleSet, nil} + badRuleSets := []MutableRuleSet{mutable, nil} for _, rs := range badRuleSets { err = s.WriteRuleSet(rs) require.Error(t, err) @@ -531,13 +532,13 @@ func TestWriteNoNamespace(t *testing.T) { require.Error(t, err) require.Nil(t, nss) - ruleSet, err := NewMutableRuleSetFromSchema(0, testRuleSet) + mutable, err := newMutableRuleSetFromSchema(0, testRuleSet) require.NoError(t, err) namespaces, err := NewNamespaces(0, testNamespaces) require.NoError(t, err) - err = s.WriteAll(&namespaces, ruleSet) + err = s.WriteAll(&namespaces, mutable) require.NoError(t, err) rs, err = s.ReadRuleSet(testNamespace) @@ -546,7 +547,7 @@ func TestWriteNoNamespace(t *testing.T) { _, err = s.ReadNamespaces() require.NoError(t, err) - err = s.WriteRuleSet(rs) + err = s.WriteRuleSet(rs.ToMutableRuleSet()) require.NoError(t, err) rs, err = s.ReadRuleSet(testNamespace)