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

[exporter/datadog] Optionally send min and max metrics for OTLP Histograms when available #20285

Merged
merged 6 commits into from
Mar 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 14 additions & 0 deletions .chloggen/mx-psi_send-aggregations-deprecation.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: deprecation

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

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Deprecate `metrics::histograms::send_count_sum_metrics` in favor of `metrics::histograms::send_aggregation_metrics`.

# One or more tracking issues related to the change
issues: [20285]

subtext: |
Toggling `metrics::histograms::send_count_sum_metrics` will now send .min and .max metrics when available.
11 changes: 11 additions & 0 deletions .chloggen/mx-psi_send-aggregations.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# 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: datadogexporter

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Send .min and .max metrics for delta OTLP Histograms and Exponential Histograms when `metrics::histograms::send_aggregation_metrics` is enabled.

# One or more tracking issues related to the change
issues: [20285]
6 changes: 3 additions & 3 deletions cmd/configschema/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ require (
require (
github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.2.0 // indirect
github.com/AzureAD/microsoft-authentication-library-for-go v0.7.0 // indirect
github.com/DataDog/opentelemetry-mapping-go/pkg/otlp/attributes v0.1.3 // indirect
github.com/DataDog/opentelemetry-mapping-go/pkg/otlp/metrics v0.1.2 // indirect
github.com/DataDog/opentelemetry-mapping-go/pkg/otlp/attributes v0.1.4 // indirect
github.com/DataDog/opentelemetry-mapping-go/pkg/otlp/metrics v0.1.4 // indirect
github.com/open-telemetry/opentelemetry-collector-contrib/exporter/alibabacloudlogserviceexporter v0.74.0 // indirect
github.com/open-telemetry/opentelemetry-collector-contrib/exporter/awscloudwatchlogsexporter v0.74.0 // indirect
github.com/open-telemetry/opentelemetry-collector-contrib/exporter/awsemfexporter v0.74.0 // indirect
Expand Down Expand Up @@ -252,7 +252,7 @@ require (
github.com/DataDog/datadog-go/v5 v5.1.1 // indirect
github.com/DataDog/go-tuf v0.3.0--fix-localmeta-fork // indirect
github.com/DataDog/gohai v0.0.0-20220718130825-1776f9beb9cc // indirect
github.com/DataDog/opentelemetry-mapping-go/pkg/quantile v0.1.3 // indirect
github.com/DataDog/opentelemetry-mapping-go/pkg/quantile v0.1.4 // indirect
github.com/DataDog/sketches-go v1.4.1 // indirect
github.com/DataDog/zstd v1.5.2 // indirect
github.com/GehirnInc/crypt v0.0.0-20200316065508-bb7000b8a962 // indirect
Expand Down
14 changes: 7 additions & 7 deletions cmd/configschema/go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions cmd/otelcontribcol/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -229,9 +229,9 @@ require (
github.com/DataDog/datadog-go/v5 v5.1.1 // indirect
github.com/DataDog/go-tuf v0.3.0--fix-localmeta-fork // indirect
github.com/DataDog/gohai v0.0.0-20220718130825-1776f9beb9cc // indirect
github.com/DataDog/opentelemetry-mapping-go/pkg/otlp/attributes v0.1.3 // indirect
github.com/DataDog/opentelemetry-mapping-go/pkg/otlp/metrics v0.1.2 // indirect
github.com/DataDog/opentelemetry-mapping-go/pkg/quantile v0.1.3 // indirect
github.com/DataDog/opentelemetry-mapping-go/pkg/otlp/attributes v0.1.4 // indirect
github.com/DataDog/opentelemetry-mapping-go/pkg/otlp/metrics v0.1.4 // indirect
github.com/DataDog/opentelemetry-mapping-go/pkg/quantile v0.1.4 // indirect
github.com/DataDog/sketches-go v1.4.1 // indirect
github.com/DataDog/zstd v1.5.2 // indirect
github.com/GehirnInc/crypt v0.0.0-20200316065508-bb7000b8a962 // indirect
Expand Down
14 changes: 7 additions & 7 deletions cmd/otelcontribcol/go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

33 changes: 28 additions & 5 deletions exporter/datadogexporter/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"go.opentelemetry.io/collector/confmap"
"go.opentelemetry.io/collector/exporter/exporterhelper"
"go.uber.org/multierr"
"go.uber.org/zap"

"github.com/open-telemetry/opentelemetry-collector-contrib/exporter/datadogexporter/internal/hostmetadata/valid"
)
Expand Down Expand Up @@ -108,20 +109,25 @@ type HistogramConfig struct {
// Mode for exporting histograms. Valid values are 'distributions', 'counters' or 'nobuckets'.
// - 'distributions' sends histograms as Datadog distributions (recommended).
// - 'counters' sends histograms as Datadog counts, one metric per bucket.
// - 'nobuckets' sends no bucket histogram metrics. .sum and .count metrics will still be sent
// if `send_count_sum_metrics` is enabled.
// - 'nobuckets' sends no bucket histogram metrics. Aggregation metrics will still be sent
// if `send_aggregation_metrics` is enabled.
//
// The current default is 'distributions'.
Mode HistogramMode `mapstructure:"mode"`

// SendCountSum states if the export should send .sum and .count metrics for histograms.
// The current default is false.
// The default is false.
// Deprecated: [v0.75.0] Use `send_aggregations` (HistogramConfig.SendAggregations) instead.
SendCountSum bool `mapstructure:"send_count_sum_metrics"`

// SendAggregations states if the exporter should send .sum, .count, .min and .max metrics for histograms.
// The default is false.
SendAggregations bool `mapstructure:"send_aggregation_metrics"`
}

func (c *HistogramConfig) validate() error {
if c.Mode == HistogramModeNoBuckets && !c.SendCountSum {
return fmt.Errorf("'nobuckets' mode and `send_count_sum_metrics` set to false will send no histogram metrics")
if c.Mode == HistogramModeNoBuckets && !c.SendAggregations {
return fmt.Errorf("'nobuckets' mode and `send_aggregation_metrics` set to false will send no histogram metrics")
}
return nil
}
Expand Down Expand Up @@ -364,6 +370,16 @@ type Config struct {
// This flag is incompatible with disabling host metadata,
// `use_resource_metadata`, or `host_metadata::hostname_source != first_resource`
OnlyMetadata bool `mapstructure:"only_metadata"`

// Non-fatal warnings found during configuration loading.
warnings []error
}

// logWarnings logs warning messages that were generated on unmarshaling.
func (c *Config) logWarnings(logger *zap.Logger) {
for _, err := range c.warnings {
logger.Warn(fmt.Sprintf("%v", err))
}
}

var _ component.Config = (*Config)(nil)
Expand Down Expand Up @@ -488,6 +504,13 @@ func (c *Config) Unmarshal(configMap *confmap.Conf) error {
return err
}

// Add deprecation warnings for deprecated settings.
renamingWarnings, err := handleRenamedSettings(configMap, c)
if err != nil {
return err
}
c.warnings = append(c.warnings, renamingWarnings...)

c.API.Key = configopaque.String(strings.TrimSpace(string(c.API.Key)))

// If an endpoint is not explicitly set, override it based on the site.
Expand Down
6 changes: 3 additions & 3 deletions exporter/datadogexporter/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,12 @@ func TestValidate(t *testing.T) {
API: APIConfig{Key: "notnull"},
Metrics: MetricsConfig{
HistConfig: HistogramConfig{
Mode: HistogramModeNoBuckets,
SendCountSum: false,
Mode: HistogramModeNoBuckets,
SendAggregations: false,
},
},
},
err: "'nobuckets' mode and `send_count_sum_metrics` set to false will send no histogram metrics",
err: "'nobuckets' mode and `send_aggregation_metrics` set to false will send no histogram metrics",
},
{
name: "TLS settings are valid",
Expand Down
85 changes: 85 additions & 0 deletions exporter/datadogexporter/config_warnings.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package datadogexporter // import "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/datadogexporter"

import (
"fmt"

"go.opentelemetry.io/collector/confmap"
"go.uber.org/multierr"
)

var _ error = (*deprecatedError)(nil)

// deprecatedError is an error related to a renamed setting.
type deprecatedError struct {
// oldName of the configuration option.
oldName string
// newName of the configuration option.
newName string
// updateFn updates the configuration to map the old value into the new one.
// It must only be called when the old value is set and is not the default.
updateFn func(*Config)
}

// List of settings that are deprecated but not yet removed.
var renamedSettings = []deprecatedError{
{
oldName: "metrics::histograms::send_count_sum_metrics",
newName: "metrics::histograms::send_aggregation_metrics",
updateFn: func(c *Config) {
c.Metrics.HistConfig.SendAggregations = c.Metrics.HistConfig.SendCountSum
},
},
}

// Error implements the error interface.
func (e deprecatedError) Error() string {
return fmt.Sprintf(
"%q has been deprecated in favor of %q",
e.oldName,
e.newName,
)
}

// Check if the deprecated option is being used.
// Error out if both the old and new options are being used.
func (e deprecatedError) Check(configMap *confmap.Conf) (bool, error) {
if configMap.IsSet(e.oldName) && configMap.IsSet(e.newName) {
return false, fmt.Errorf("%q and %q can't be both set at the same time: use %q only instead", e.oldName, e.newName, e.newName)
}
return configMap.IsSet(e.oldName), nil
}

// UpdateCfg to move the old configuration value into the new one.
func (e deprecatedError) UpdateCfg(cfg *Config) {
e.updateFn(cfg)
}

// handleRenamedSettings for a given configuration map.
// Error out if any pair of old-new options are set at the same time.
func handleRenamedSettings(configMap *confmap.Conf, cfg *Config) (warnings []error, err error) {
for _, renaming := range renamedSettings {
isOldNameUsed, errCheck := renaming.Check(configMap)
err = multierr.Append(err, errCheck)

if errCheck == nil && isOldNameUsed {
warnings = append(warnings, renaming)
// only update config if old name is in use
renaming.UpdateCfg(cfg)
}
}
return
}
107 changes: 107 additions & 0 deletions exporter/datadogexporter/config_warnings_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package datadogexporter

import (
"testing"

"github.com/stretchr/testify/assert"
"go.opentelemetry.io/collector/confmap"
)

func TestSendAggregations(t *testing.T) {
tests := []struct {
name string
cfgMap *confmap.Conf
expectedAggrValue bool
warnings []string
err string
}{
{
name: "both metrics::histograms::send_count_sum_metrics and metrics::histograms::send_aggregation_metrics",
cfgMap: confmap.NewFromStringMap(map[string]any{
"metrics": map[string]any{
"histograms": map[string]any{
"send_count_sum_metrics": true,
"send_aggregation_metrics": true,
},
},
}),
err: "\"metrics::histograms::send_count_sum_metrics\" and \"metrics::histograms::send_aggregation_metrics\" can't be both set at the same time: use \"metrics::histograms::send_aggregation_metrics\" only instead",
},
{
name: "metrics::histograms::send_count_sum_metrics set to true",
cfgMap: confmap.NewFromStringMap(map[string]any{
"metrics": map[string]any{
"histograms": map[string]any{
"send_count_sum_metrics": true,
},
},
}),
expectedAggrValue: true,
warnings: []string{
"\"metrics::histograms::send_count_sum_metrics\" has been deprecated in favor of \"metrics::histograms::send_aggregation_metrics\"",
},
},
{
name: "metrics::histograms::send_count_sum_metrics set to false",
cfgMap: confmap.NewFromStringMap(map[string]any{
"metrics": map[string]any{
"histograms": map[string]any{
"send_count_sum_metrics": false,
},
},
}),
warnings: []string{
"\"metrics::histograms::send_count_sum_metrics\" has been deprecated in favor of \"metrics::histograms::send_aggregation_metrics\"",
},
},
{
name: "metrics::histograms::send_count_sum_metrics and metrics::histograms::send_aggregation_metrics unset",
cfgMap: confmap.New(),
expectedAggrValue: false,
},
{
name: "metrics::histograms::send_aggregation_metrics set",
cfgMap: confmap.NewFromStringMap(map[string]any{
"metrics": map[string]any{
"histograms": map[string]any{
"send_aggregation_metrics": true,
},
},
}),
expectedAggrValue: true,
},
}

for _, testInstance := range tests {
t.Run(testInstance.name, func(t *testing.T) {
f := NewFactory()
cfg := f.CreateDefaultConfig().(*Config)
err := cfg.Unmarshal(testInstance.cfgMap)
if err != nil || testInstance.err != "" {
assert.EqualError(t, err, testInstance.err)
} else {
assert.Equal(t, testInstance.expectedAggrValue, cfg.Metrics.HistConfig.SendAggregations)
var warningStr []string
for _, warning := range cfg.warnings {
warningStr = append(warningStr, warning.Error())
}
assert.ElementsMatch(t, testInstance.warnings, warningStr)
}
})
}

}
Loading