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.

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

[prometheusremotewrite] add otel_scope_info metric to prometheus remote write exporter #24248

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
07e26dd
add otel_scope_info metric to prometheus remote write exporter
blakeroberts-wk Jul 12, 2023
c0c0acd
default otel_scope_info metric generation to false for prometheus rem…
blakeroberts-wk Jul 17, 2023
000bb0b
Merge branch 'main' into O11Y-3152
blakeroberts-wk Jul 24, 2023
b7a328b
fix lints and tests
blakeroberts-wk Jul 24, 2023
57840c1
Merge branch 'main' into O11Y-3152
blakeroberts-wk Aug 1, 2023
dabcd5d
add changelog entry
blakeroberts-wk Aug 1, 2023
8814c5a
Merge branch 'main' into O11Y-3152
blakeroberts-wk Aug 2, 2023
3f146d9
emit otel_scope_info metric when scope doesn't contain attributes
blakeroberts-wk Aug 14, 2023
75864aa
Merge branch 'main' into O11Y-3152
blakeroberts-wk Oct 13, 2023
9cef3ce
drop otel_scope_info metric when scope attributes are empty
blakeroberts-wk Oct 13, 2023
a50c3ef
remove unnecessary resource copy
blakeroberts-wk Oct 16, 2023
91b40e2
Merge branch 'main' into O11Y-3152
blakeroberts-wk Oct 17, 2023
d47d532
add scope name/version to prometheus remote write helpers unit test
blakeroberts-wk Oct 18, 2023
b26d514
Merge branch 'main' into O11Y-3152
blakeroberts-wk Oct 25, 2023
465443b
Merge branch 'main' into O11Y-3152
blakeroberts-wk Oct 26, 2023
32f2736
Merge branch 'main' into O11Y-3152
blakeroberts-wk Nov 2, 2023
a7a231c
Merge branch 'main' into O11Y-3152
blakeroberts-wk Nov 10, 2023
8e36f96
Merge branch 'main' into O11Y-3152
blakeroberts-wk Nov 16, 2023
4c1546a
Merge branch 'main' into O11Y-3152
blakeroberts-wk Nov 28, 2023
9250192
Merge branch 'main' into O11Y-3152
blakeroberts-wk Dec 11, 2023
8a0eb23
Merge branch 'main' into O11Y-3152
blakeroberts-wk Dec 27, 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
20 changes: 20 additions & 0 deletions .chloggen/O11Y-3152.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Use this changelog template to create an entry for release notes.
# If your change doesn't affect end users, such as a test fix or a tooling change,
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: enhancement

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: prometheusremotewriteexporter

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Add otel_scope_info metric to prometheus remote write exporter.
blakeroberts-wk marked this conversation as resolved.
Show resolved Hide resolved

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [21091]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:
2 changes: 2 additions & 0 deletions exporter/prometheusremotewriteexporter/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ The following settings can be optionally configured:
- `enabled` (default = false): If `enabled` is `true`, all the resource attributes will be converted to metric labels by default.
- `target_info`: customize `target_info` metric
- `enabled` (default = true): If `enabled` is `true`, a `target_info` metric will be generated for each resource metric (see https://github.com/open-telemetry/opentelemetry-specification/pull/2381).
- `scope_info`: customize `otel_scope_info` metric
- `enabled` (default = false): If `enabled` is `true`, a `otel_scope_info` metric will be generated for each instrumentation scope metric (see https://github.com/open-telemetry/opentelemetry-specification/pull/2703).
- `export_created_metric`:
- `enabled` (default = false): If `enabled` is `true`, a `_created` metric is
exported for Summary, Histogram, and Monotonic Sum metric points if
Expand Down
8 changes: 8 additions & 0 deletions exporter/prometheusremotewriteexporter/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ type Config struct {
// TargetInfo allows customizing the target_info metric
TargetInfo *TargetInfo `mapstructure:"target_info,omitempty"`

// ScopeInfo allows customizing the scope_info metric
ScopeInfo *ScopeInfo `mapstructure:"scope_info,omitempty"`

// CreatedMetric allows customizing creation of _created metrics
CreatedMetric *CreatedMetric `mapstructure:"export_created_metric,omitempty"`

Expand All @@ -57,6 +60,11 @@ type TargetInfo struct {
Enabled bool `mapstructure:"enabled"`
}

type ScopeInfo struct {
// Enabled if false the otel_scope_info metric is not generated by the exporter
Enabled bool `mapstructure:"enabled"`
}

// RemoteWriteQueue allows to configure the remote write queue.
type RemoteWriteQueue struct {
// Enabled if false the queue is not enabled, the export requests
Expand Down
3 changes: 3 additions & 0 deletions exporter/prometheusremotewriteexporter/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@ func TestLoadConfig(t *testing.T) {
TargetInfo: &TargetInfo{
Enabled: true,
},
ScopeInfo: &ScopeInfo{
Enabled: false,
},
CreatedMetric: &CreatedMetric{Enabled: true},
},
},
Expand Down
1 change: 1 addition & 0 deletions exporter/prometheusremotewriteexporter/exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ func newPRWExporter(cfg *Config, set exporter.CreateSettings) (*prwExporter, err
Namespace: cfg.Namespace,
ExternalLabels: sanitizedLabels,
DisableTargetInfo: !cfg.TargetInfo.Enabled,
DisableScopeInfo: !cfg.ScopeInfo.Enabled,
ExportCreatedMetric: cfg.CreatedMetric.Enabled,
AddMetricSuffixes: cfg.AddMetricSuffixes,
},
Expand Down
9 changes: 9 additions & 0 deletions exporter/prometheusremotewriteexporter/exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ func Test_NewPRWExporter(t *testing.T) {
TargetInfo: &TargetInfo{
Enabled: true,
},
ScopeInfo: &ScopeInfo{
Enabled: false,
},
CreatedMetric: &CreatedMetric{
Enabled: false,
},
Expand Down Expand Up @@ -136,6 +139,9 @@ func Test_Start(t *testing.T) {
TargetInfo: &TargetInfo{
Enabled: true,
},
ScopeInfo: &ScopeInfo{
Enabled: false,
},
CreatedMetric: &CreatedMetric{
Enabled: false,
},
Expand Down Expand Up @@ -679,6 +685,9 @@ func Test_PushMetrics(t *testing.T) {
TargetInfo: &TargetInfo{
Enabled: true,
},
ScopeInfo: &ScopeInfo{
Enabled: false,
},
CreatedMetric: &CreatedMetric{
Enabled: true,
},
Expand Down
3 changes: 3 additions & 0 deletions exporter/prometheusremotewriteexporter/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@ func createDefaultConfig() component.Config {
TargetInfo: &TargetInfo{
Enabled: true,
},
ScopeInfo: &ScopeInfo{
Enabled: false,
},
CreatedMetric: &CreatedMetric{
Enabled: false,
},
Expand Down
83 changes: 75 additions & 8 deletions pkg/translator/prometheusremotewrite/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ const (
spanIDKey = "span_id"
infoType = "info"
targetMetricName = "target_info"
scopeMetricName = "otel_scope_info"
scopeAttrName = "otel_scope_name"
scopeAttrVersion = "otel_scope_version"
)

type bucketBoundsData struct {
Expand Down Expand Up @@ -155,7 +158,13 @@ func timeSeriesSignature(datatype string, labels *[]prompb.Label) string {
// createAttributes creates a slice of Cortex Label with OTLP attributes and pairs of string values.
// Unpaired string value is ignored. String pairs overwrites OTLP labels if collision happens, and the overwrite is
// logged. Resultant label names are sanitized.
func createAttributes(resource pcommon.Resource, attributes pcommon.Map, externalLabels map[string]string, extras ...string) []prompb.Label {
func createAttributes(
resource pcommon.Resource,
scope pcommon.InstrumentationScope,
attributes pcommon.Map,
externalLabels map[string]string,
extras ...string,
) []prompb.Label {
serviceName, haveServiceName := resource.Attributes().Get(conventions.AttributeServiceName)
instance, haveInstanceID := resource.Attributes().Get(conventions.AttributeServiceInstanceID)

Expand Down Expand Up @@ -212,6 +221,14 @@ func createAttributes(resource pcommon.Resource, attributes pcommon.Map, externa
l[key] = value
}

// add scope name/version labels
if len(scope.Name()) > 0 {
extras = append(extras, scopeAttrName, scope.Name())
}
if len(scope.Version()) > 0 {
blakeroberts-wk marked this conversation as resolved.
Show resolved Hide resolved
extras = append(extras, scopeAttrVersion, scope.Version())
}

for i := 0; i < len(extras); i += 2 {
if i+1 >= len(extras) {
break
Expand Down Expand Up @@ -254,11 +271,18 @@ func isValidAggregationTemporality(metric pmetric.Metric) bool {

// addSingleHistogramDataPoint converts pt to 2 + min(len(ExplicitBounds), len(BucketCount)) + 1 samples. It
// ignore extra buckets if len(ExplicitBounds) > len(BucketCounts)
func addSingleHistogramDataPoint(pt pmetric.HistogramDataPoint, resource pcommon.Resource, metric pmetric.Metric, settings Settings, tsMap map[string]*prompb.TimeSeries) {
func addSingleHistogramDataPoint(
pt pmetric.HistogramDataPoint,
resource pcommon.Resource,
scope pcommon.InstrumentationScope,
metric pmetric.Metric,
settings Settings,
tsMap map[string]*prompb.TimeSeries,
) {
timestamp := convertTimeStamp(pt.Timestamp())
// sum, count, and buckets of the histogram should append suffix to baseName
baseName := prometheustranslator.BuildCompliantName(metric, settings.Namespace, settings.AddMetricSuffixes)
baseLabels := createAttributes(resource, pt.Attributes(), settings.ExternalLabels)
baseLabels := createAttributes(resource, scope, pt.Attributes(), settings.ExternalLabels)

createLabels := func(nameSuffix string, extras ...string) []prompb.Label {
extraLabelCount := len(extras) / 2
Expand Down Expand Up @@ -288,7 +312,6 @@ func addSingleHistogramDataPoint(pt pmetric.HistogramDataPoint, resource pcommon

sumlabels := createLabels(sumStr)
addSample(tsMap, sum, sumlabels, metric.Type().String())

}

// treat count as a sample in an individual TimeSeries
Expand Down Expand Up @@ -452,12 +475,18 @@ func maxTimestamp(a, b pcommon.Timestamp) pcommon.Timestamp {
}

// addSingleSummaryDataPoint converts pt to len(QuantileValues) + 2 samples.
func addSingleSummaryDataPoint(pt pmetric.SummaryDataPoint, resource pcommon.Resource, metric pmetric.Metric, settings Settings,
tsMap map[string]*prompb.TimeSeries) {
func addSingleSummaryDataPoint(
pt pmetric.SummaryDataPoint,
resource pcommon.Resource,
scope pcommon.InstrumentationScope,
metric pmetric.Metric,
settings Settings,
tsMap map[string]*prompb.TimeSeries,
) {
timestamp := convertTimeStamp(pt.Timestamp())
// sum and count of the summary should append suffix to baseName
baseName := prometheustranslator.BuildCompliantName(metric, settings.Namespace, settings.AddMetricSuffixes)
baseLabels := createAttributes(resource, pt.Attributes(), settings.ExternalLabels)
baseLabels := createAttributes(resource, scope, pt.Attributes(), settings.ExternalLabels)

createLabels := func(name string, extras ...string) []prompb.Label {
extraLabelCount := len(extras) / 2
Expand Down Expand Up @@ -566,7 +595,7 @@ func addResourceTargetInfo(resource pcommon.Resource, settings Settings, timesta
if len(settings.Namespace) > 0 {
name = settings.Namespace + "_" + name
}
labels := createAttributes(resource, attributes, settings.ExternalLabels, nameStr, name)
labels := createAttributes(resource, pcommon.NewInstrumentationScope(), attributes, settings.ExternalLabels, nameStr, name)
sample := &prompb.Sample{
Value: float64(1),
// convert ns to ms
Expand All @@ -575,6 +604,44 @@ func addResourceTargetInfo(resource pcommon.Resource, settings Settings, timesta
addSample(tsMap, sample, labels, infoType)
}

func addScopeTargetInfo(
blakeroberts-wk marked this conversation as resolved.
Show resolved Hide resolved
scope pcommon.InstrumentationScope,
resource pcommon.Resource,
settings Settings,
timestamp pcommon.Timestamp,
tsMap map[string]*prompb.TimeSeries,
) {
if settings.DisableScopeInfo {
return
}
if scope.Attributes().Len() == 0 {
// If the scope doesn't have additional attributes, then otel_scope_info isn't useful.
Copy link
Contributor

Choose a reason for hiding this comment

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

It will contain the name and version of the instrumentation scope. Isn't this information per se useful?

It will contain the name and version of the instrumentation scope. Isn't this information per se useful? It means that there is at least a single metric being emitted in that scope.

Having scope_info in the configuration enabled and not generating the otel_scope_info metric is misleading and will confuse users.

Copy link
Contributor

Choose a reason for hiding this comment

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

looking into the compliance matrix, it seems that only PHP and rust will be able to set the scope using the Otel API?

get_meter accepts attributes.

So one more point to for users to have the least surprise by always emitting this metric?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't this information per se useful? It means that there is at least a single metric being emitted in that scope.

If one was looking for metrics under a specific scope, they'd probably do something like a topk query on

__name__{otel_scope_name="w/e"}

In general, actual usage of otel_scope_info is most likely after knowing the __name__ you're after then

<query> / on (otel_scope_name, otel_scope_version) group_left otel_scope_info

Though the query would return no data if otel_scope_info doesn't exist. So if the feature is enabled, it might make sense to always report it so queries that use the metric always return data regardless of whether or not the joined labels add novel information

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 pushed a commit that emitted the metric regardless.

But now I'm second guessing it. This has the potential to add a lot of active time series that won't be valuable the majority of the time. The collector could receive metrics from many languages and only PHP and rust even have the potential to make this metric add novel information. And the query with the join tends to be a rather advanced use case (so advanced that documentation doesn't exist around how to properly use otel_scope_info let alone target_info) Additionally, not emitting the metric if it doesn't have attributes would align with the implementation of target_info: https://github.com/blakeroberts-wk/opentelemetry-collector-contrib/blob/3f146d90bd9c622c33359c71fcae89162a263f2c/pkg/translator/prometheusremotewrite/helper.go#L589-L591

return
}

// Only add service name and instance id resource attributes
resCopy := pcommon.NewResource()
serviceName, ok := resource.Attributes().Get(conventions.AttributeServiceName)
if ok {
serviceName.CopyTo(resCopy.Attributes().PutEmpty(conventions.AttributeServiceName))
}
serviceInstanceID, ok := resource.Attributes().Get(conventions.AttributeServiceInstanceID)
if ok {
serviceInstanceID.CopyTo(resCopy.Attributes().PutEmpty(conventions.AttributeServiceInstanceID))
}

name := scopeMetricName
if len(settings.Namespace) > 0 {
name = settings.Namespace + "_" + name
}
labels := createAttributes(resource, scope, scope.Attributes(), settings.ExternalLabels, nameStr, name)
blakeroberts-wk marked this conversation as resolved.
Show resolved Hide resolved
sample := &prompb.Sample{
Value: float64(1),
Timestamp: convertTimeStamp(timestamp),
}
addSample(tsMap, sample, labels, infoType)
}

// convertTimeStamp converts OTLP timestamp in ns to timestamp in ms
func convertTimeStamp(timestamp pcommon.Timestamp) int64 {
return timestamp.AsTime().UnixNano() / (int64(time.Millisecond) / int64(time.Nanosecond))
Expand Down
Loading