Skip to content

Commit

Permalink
cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
thampiotr committed Jan 28, 2025
1 parent e1f9bab commit b370795
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 14 deletions.
1 change: 0 additions & 1 deletion internal/component/discovery/discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,6 @@ func toAlloyTargets(cache map[string]*targetgroup.Group) []Target {

for _, group := range cache {
for _, target := range group.Targets {
// TODO(thampiotr): Merge 20% allocs
allTargets = append(allTargets, NewTargetFromSpecificAndBaseLabelSet(target, group.Labels))
}
}
Expand Down
16 changes: 6 additions & 10 deletions internal/component/discovery/target.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ type Target struct {
// Prometheus codebase (even for logs, we have loki.relabel) and this representation helps reduce
// unnecessary conversions and allocations. We can add another internal representations in the future if needed.
labelSet commonlabels.LabelSet
// NOTE: it is essential that equality between targets continues to work as it is used by Alloy runtime to
// decide whether updates need to be propagated throughout the pipeline. See tests.
}

var (
Expand All @@ -40,7 +42,7 @@ func NewTargetFromLabelSet(targetLabels commonlabels.LabelSet) Target {
}
}

// TODO(thampiotr): 37% allocs
// TODO(thampiotr): 27% allocs
// TODO(thampiotr): discovery.*
func NewTargetFromSpecificAndBaseLabelSet(specific, base commonlabels.LabelSet) Target {
merged := make(commonlabels.LabelSet, len(specific)+len(base))
Expand All @@ -53,7 +55,7 @@ func NewTargetFromSpecificAndBaseLabelSet(specific, base commonlabels.LabelSet)
return NewTargetFromLabelSet(merged)
}

// TODO(thampiotr): 23% allocs
// TODO(thampiotr): 27% allocs
// TODO(thampiotr): discovery.relabel
func NewTargetFromModelLabels(labels modellabels.Labels) Target {
// TODO(thampiotr): save labels as cached value?
Expand All @@ -74,14 +76,14 @@ func NewTargetFromMap(m map[string]string) Target {
return NewTargetFromLabelSet(l)
}

// TODO(thampiotr): 12% allocs
// TODO(thampiotr): 13% allocs
// TODO(thampiotr): discovery.relabel
func (t Target) Labels() modellabels.Labels {
// TODO(thampiotr): We can cache this!
return t.LabelsWithPredicate(nil)
}

// TODO(thampiotr): 11% allocs
// TODO(thampiotr): 13% allocs
// TODO(thampiotr): prometheus.scrape / distributed_targets
func (t Target) NonMetaLabels() modellabels.Labels {
return t.LabelsWithPredicate(func(key string) bool {
Expand Down Expand Up @@ -174,12 +176,6 @@ func (t Target) Len() int {
return len(t.labelSet)
}

// Equals should be called to compare two Target objects.
// TODO(thampiotr): make sure this is called when Alloy is deciding whether to propagate updates
func (t Target) Equals(other Target) bool {
return t.labelSet.Equal(other.labelSet)
}

func (t Target) Clone() Target {
return Target{
labelSet: t.labelSet.Clone(),
Expand Down
4 changes: 2 additions & 2 deletions internal/component/discovery/target_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func TestDecodeMap(t *testing.T) {
require.NoError(t, eval.Evaluate(scope, &actual))
require.Equal(t, expected, actual)

// Test can use it like a map
// Test can iterate over it
var seen []string
actual.ForEachLabel(func(k string, v string) bool {
seen = append(seen, fmt.Sprintf("%s=%s", k, v))
Expand Down Expand Up @@ -84,8 +84,8 @@ func TestConvertFromNative(t *testing.T) {
}

func TestEquals(t *testing.T) {
// NOTE: if we start caching anything as a field, the equality may break. We should test it.
t1 := NewTargetFromMap(map[string]string{"hip": "hop", "boom": "bap"})
// TODO(thampiotr): if we start caching this as a field, the equality may break.
require.Equal(t, 2, t1.Labels().Len())
t2 := NewTargetFromMap(map[string]string{"hip": "hop"})
t2.Set("boom", "bap")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,6 @@ func (cn *BuiltinComponentNode) setExports(e component.Exports) {
var changed bool

cn.exportsMut.Lock()
// TODO(thampiotr): If we export a capsule and don't want to trigger updates, we would want to have a customisable way of comparing them to each other here.
if !reflect.DeepEqual(cn.exports, e) {
changed = true
cn.exports = e
Expand Down

0 comments on commit b370795

Please sign in to comment.