Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add [default] option to default the project_id label for exemplars #449

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Add [default] option to default the project_id label for exemplars #449

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ac9537f
add new flags and exporter opts for defaulting the exemplar project_i…
realschwa Mar 22, 2023
27d44cd
add exemplarOpts to autopopulate the project_id label for exemplars i…
realschwa Mar 23, 2023
fa7f1b7
add exemplarOpts to unit tests for transform_test.go
realschwa Mar 23, 2023
4cd8efa
add unit test
realschwa Mar 24, 2023
af70e76
fix unit test
realschwa Mar 24, 2023
1542c50
address flag comment
realschwa Mar 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion pkg/export/export.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,12 @@ type ExporterOpts struct {
Location string
Cluster string

// If true, automatically populate the project id in an exemplar labelset.
PopulateExemplarProjectID bool
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say let's avoid bools flag. They are not flexible and scalable. What about empty ExemplarProjectID stopping this to populate that extra label?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the semantics of this would get confusing since this is supposed to be both overridable and disableable.

An empty ExemplarProjectID seems to make sense as the case where you would not autopopulate the project_id, but not passing something in seems to imply that it is the default state which is not what we want. We want the default state to be to autopopulate the exemplar projectID.

Unless you mean empty as in the literal empty string ("") which also seems to be semantically confusing.

I'll defer to you, @pintohutch and @lyanco on this one.

// Automatically populate exemplar labelset with this project id
// if PopulateExemplarProjectID is true.
ExemplarProjectID string

// A list of metric matchers. Only Prometheus time series satisfying at
// least one of the matchers are exported.
// This option matches the semantics of the Prometheus federation match[]
Expand Down Expand Up @@ -420,7 +426,11 @@ func (e *Exporter) Export(metadata MetadataFunc, batch []record.RefSample, exemp
samplesDropped.WithLabelValues("no-ha-range").Add(float64(batchSize))
return
}
builder := newSampleBuilder(e.seriesCache)
exOpts := &exemplarOpts{
autoPopulateProjectID: e.opts.PopulateExemplarProjectID,
exemplarProjectID: e.opts.ExemplarProjectID,
}
builder := newSampleBuilder(e.seriesCache, exOpts)
defer builder.close()
exemplarsExported.Add(float64(len(exemplarMap)))

Expand Down
6 changes: 6 additions & 0 deletions pkg/export/setup/setup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,12 @@ func FromFlags(a *kingpin.Application, userAgentProduct string) func(log.Logger,
a.Flag("export.label.project-id", fmt.Sprintf("Default project ID set for all exported data. Prefer setting the external label %q in the Prometheus configuration if not using the auto-discovered default.", export.KeyProjectID)).
Default(opts.ProjectID).StringVar(&opts.ProjectID)

a.Flag("export.exemplars.populate-exemplar-project-id", "If true, automatically populate the 'project_id' label in an exemplar labelset if a 'span_id' and 'trace_id' are also present.").
Default("true").BoolVar(&opts.PopulateExemplarProjectID)

a.Flag("export.exemplars.project-id", "If non-empty and export.exemplars.populate-exemplar-project-id is true, use this project_id as the project_id for exemplar labelsets.").
realschwa marked this conversation as resolved.
Show resolved Hide resolved
Default(opts.ProjectID).StringVar(&opts.ExemplarProjectID)

a.Flag("export.user-agent-mode", fmt.Sprintf("Mode for user agent used for requests against the GCM API. Valid values are %q, %q, %q, %q or %q.", UAModeGKE, UAModeKubectl, UAModeAVMW, UAModeABM, UAModeUnspecified)).
Default("unspecified").EnumVar(&opts.UserAgentMode, UAModeUnspecified, UAModeGKE, UAModeKubectl, UAModeAVMW, UAModeABM)

Expand Down
28 changes: 21 additions & 7 deletions pkg/export/transform.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,15 +69,24 @@ func discardExemplarIncIfExists(series storage.SeriesRef, exemplars map[storage.
}
}

type exemplarOpts struct {
autoPopulateProjectID bool
exemplarProjectID string
}
type sampleBuilder struct {
series *seriesCache
dists map[uint64]*distribution
exOpts *exemplarOpts
}

func newSampleBuilder(c *seriesCache) *sampleBuilder {
func newSampleBuilder(c *seriesCache, exOpts *exemplarOpts) *sampleBuilder {
if exOpts == nil {
exOpts = &exemplarOpts{}
}
return &sampleBuilder{
series: c,
dists: make(map[uint64]*distribution, 128),
exOpts: exOpts,
}
}

Expand Down Expand Up @@ -270,7 +279,7 @@ func (d *distribution) Swap(i, j int) {
d.values[i], d.values[j] = d.values[j], d.values[i]
}

func (d *distribution) build(lset labels.Labels) (*distribution_pb.Distribution, error) {
func (d *distribution) build(lset labels.Labels, exOpts *exemplarOpts) (*distribution_pb.Distribution, error) {
// The exposition format in general requires buckets to be in-order but we observed
// some cases in the wild where this was not the case.
// Ensure sorting here to gracefully handle those cases sometimes. This cannot handle
Expand Down Expand Up @@ -354,7 +363,7 @@ func (d *distribution) build(lset labels.Labels) (*distribution_pb.Distribution,
},
},
BucketCounts: values,
Exemplars: buildExemplars(d.exemplars),
Exemplars: buildExemplars(d.exemplars, exOpts),
}
return dp, nil
}
Expand Down Expand Up @@ -467,7 +476,7 @@ Loop:
if !dist.complete() {
continue
}
dp, err := dist.build(e.lset)
dp, err := dist.build(e.lset, b.exOpts)
if err != nil {
return nil, 0, samples[consumed:], err
}
Expand All @@ -482,15 +491,15 @@ Loop:
return nil, 0, samples[consumed:], nil
}

func buildExemplars(exemplars []record.RefExemplar) []*distribution_pb.Distribution_Exemplar {
func buildExemplars(exemplars []record.RefExemplar, exOpts *exemplarOpts) []*distribution_pb.Distribution_Exemplar {
// The exemplars field of a distribution value field must be in increasing order of value
// (https://cloud.google.com/monitoring/api/ref_v3/rpc/google.api#distribution) -- let's sort them.
sort.Slice(exemplars, func(i, j int) bool {
return exemplars[i].V < exemplars[j].V
})
var result []*distribution_pb.Distribution_Exemplar
for _, pex := range exemplars {
attachments := buildExemplarAttachments(pex.Labels)
attachments := buildExemplarAttachments(pex.Labels, exOpts)
result = append(result, &distribution_pb.Distribution_Exemplar{
Value: pex.V,
Timestamp: getTimestamp(pex.T),
Expand All @@ -511,7 +520,7 @@ func buildExemplars(exemplars []record.RefExemplar) []*distribution_pb.Distribut
// This is to maintain comptability with CloudTrace.
// Note that the project_id needs to be the project_id where the span was written.
// This may not necessarily be the same project_id where the metric was written.
func buildExemplarAttachments(lset labels.Labels) []*anypb.Any {
func buildExemplarAttachments(lset labels.Labels, exOpts *exemplarOpts) []*anypb.Any {
var projectID, spanID, traceID string
var attachments []*anypb.Any
droppedLabels := make(map[string]string)
Expand All @@ -526,6 +535,11 @@ func buildExemplarAttachments(lset labels.Labels) []*anypb.Any {
droppedLabels[label.Name] = label.Value
}
}
// If the project_id is not in the labelset, and autoPopulateProjectID is true,
// autopopulate projectID, but only if spanID and traceID are set.
if projectID == "" && spanID != "" && traceID != "" && exOpts.autoPopulateProjectID {
projectID = exOpts.exemplarProjectID
}
if projectID != "" && spanID != "" && traceID != "" {
spanCtx, err := anypb.New(&monitoring_pb.SpanContext{
SpanName: fmt.Sprintf(spanContextFormat, projectID, traceID, spanID),
Expand Down
143 changes: 142 additions & 1 deletion pkg/export/transform_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ func TestSampleBuilder(t *testing.T) {
matchers Matchers
wantSeries []*monitoring_pb.TimeSeries
wantFail bool
exOpts *exemplarOpts
}{
{
doc: "convert gauge",
Expand Down Expand Up @@ -1475,6 +1476,146 @@ func TestSampleBuilder(t *testing.T) {
},
},
},
{
doc: "convert histogram with exemplars project_id default",
metadata: testMetadataFunc(metricMetadataMap{
"metric1": {Type: textparse.MetricTypeHistogram, Help: "metric1 help text"},
"metric1_a_count": {Type: textparse.MetricTypeGauge, Help: "metric1_a_count help text"},
}),
series: seriesMap{
1: labels.FromStrings("job", "job1", "instance", "instance1", "__name__", "metric1_sum"),
2: labels.FromStrings("job", "job1", "instance", "instance1", "__name__", "metric1_count"),
3: labels.FromStrings("job", "job1", "instance", "instance1", "__name__", "metric1_bucket", "le", "0.1"),
4: labels.FromStrings("job", "job1", "instance", "instance1", "__name__", "metric1_bucket", "le", "0.5"),
5: labels.FromStrings("job", "job1", "instance", "instance1", "__name__", "metric1_bucket", "le", "1"),
6: labels.FromStrings("job", "job1", "instance", "instance1", "__name__", "metric1_bucket", "le", "2.5"),
7: labels.FromStrings("job", "job1", "instance", "instance1", "__name__", "metric1_bucket", "le", "+Inf"),
},
samples: [][]record.RefSample{
// First sample set, should be skipped by reset handling.
// The buckets must be in ascending order for an individual histogram but otherwise
// no order or grouping constraints apply for series of a given histogram metric.
{
{Ref: 1, T: 1000, V: 55.1}, // hist1, sum
{Ref: 3, T: 1000, V: 2}, // hist1, 0.1
{Ref: 4, T: 1000, V: 5}, // hist1, 0.5
{Ref: 5, T: 1000, V: 6}, // hist1, 1
{Ref: 6, T: 1000, V: 8}, // hist1, 2.5
{Ref: 7, T: 1000, V: 10}, // hist1, inf
{Ref: 2, T: 1000, V: 10}, // hist1, count
},
// Second sample set should actually be emitted.
{
// Second samples for histograms should produce a distribution.
{Ref: 3, T: 2000, V: 4}, // hist1, 0.1
{Ref: 2, T: 2000, V: 21}, // hist1, count
{Ref: 1, T: 2000, V: 123.4}, // hist1, sum
{Ref: 4, T: 2000, V: 9}, // hist1, 0.5
{Ref: 5, T: 2000, V: 11}, // hist1, 1
{Ref: 6, T: 2000, V: 15}, // hist1, 2.5
{Ref: 7, T: 2000, V: 21}, // hist1, inf
},
},
exemplars: []map[storage.SeriesRef]record.RefExemplar{
// First sample set is skipped by reset handling.
{},
{
3: {Ref: 3, T: 1500, V: .099, Labels: labels.New(
labels.Label{Name: "trace_id", Value: "2"},
labels.Label{Name: "span_id", Value: "3"},
)},
4: {Ref: 4, T: 1500, V: .4, Labels: labels.New(
labels.Label{Name: "project_id", Value: "explicit-project-id"},
labels.Label{Name: "trace_id", Value: "2"},
labels.Label{Name: "span_id", Value: "3"},
)},
5: {Ref: 5, T: 1500, V: .99},
6: {Ref: 6, T: 1500, V: 2},
7: {Ref: 7, T: 1500, V: 11},
},
},
exOpts: &exemplarOpts{
autoPopulateProjectID: true,
exemplarProjectID: "exemplar-project-id",
},
wantSeries: []*monitoring_pb.TimeSeries{
// 0: skipped by reset handling.
{ // 1
Resource: &monitoredres_pb.MonitoredResource{
Type: "prometheus_target",
Labels: map[string]string{
"project_id": "example-project",
"location": "europe",
"cluster": "foo-cluster",
"namespace": "",
"job": "job1",
"instance": "instance1",
},
},
Metric: &metric_pb.Metric{
Type: "prometheus.googleapis.com/metric1/histogram",
Labels: map[string]string{},
},
MetricKind: metric_pb.MetricDescriptor_CUMULATIVE,
ValueType: metric_pb.MetricDescriptor_DISTRIBUTION,
Points: []*monitoring_pb.Point{{
Interval: &monitoring_pb.TimeInterval{
StartTime: &timestamp_pb.Timestamp{Seconds: 1},
EndTime: &timestamp_pb.Timestamp{Seconds: 2},
},
Value: &monitoring_pb.TypedValue{
Value: &monitoring_pb.TypedValue_DistributionValue{
DistributionValue: &distribution_pb.Distribution{
Count: 11,
Mean: 6.20909090909091,
SumOfSquaredDeviation: 270.301590909091,
BucketOptions: &distribution_pb.Distribution_BucketOptions{
Options: &distribution_pb.Distribution_BucketOptions_ExplicitBuckets{
ExplicitBuckets: &distribution_pb.Distribution_BucketOptions_Explicit{
Bounds: []float64{0.1, 0.5, 1, 2.5},
},
},
},
BucketCounts: []int64{2, 2, 1, 2, 4},
Exemplars: []*distribution_pb.Distribution_Exemplar{
{
Value: .099,
Timestamp: &timestamp_pb.Timestamp{Seconds: 1, Nanos: 500000000},
Attachments: []*anypb.Any{
wrapAsAny(&monitoring_pb.SpanContext{
SpanName: "projects/exemplar-project-id/traces/2/spans/3",
}),
},
},
{
Value: .4,
Timestamp: &timestamp_pb.Timestamp{Seconds: 1, Nanos: 500000000},
Attachments: []*anypb.Any{
wrapAsAny(&monitoring_pb.SpanContext{
SpanName: "projects/explicit-project-id/traces/2/spans/3",
}),
},
},
{
Value: .99,
Timestamp: &timestamp_pb.Timestamp{Seconds: 1, Nanos: 500000000},
},
{
Value: 2,
Timestamp: &timestamp_pb.Timestamp{Seconds: 1, Nanos: 500000000},
},
{
Value: 11,
Timestamp: &timestamp_pb.Timestamp{Seconds: 1, Nanos: 500000000},
},
},
},
},
},
}},
},
},
},
}

for i, c := range cases {
Expand All @@ -1489,7 +1630,7 @@ func TestSampleBuilder(t *testing.T) {
var result []*monitoring_pb.TimeSeries

for i, batch := range c.samples {
b := newSampleBuilder(cache)
b := newSampleBuilder(cache, c.exOpts)

for k := 0; len(batch) > 0; k++ {
var exemplars map[storage.SeriesRef]record.RefExemplar
Expand Down