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 version 0.32.0 and higher is not compatible with opentelemetry-collector-contrib #3394

Closed
krupyansky opened this issue Oct 25, 2022 · 5 comments · Fixed by #3436
Closed
Assignees
Labels
area:metrics Part of OpenTelemetry Metrics bug Something isn't working

Comments

@krupyansky
Copy link

krupyansky commented Oct 25, 2022

Description

The library opentelemetry-collector-contrib along the path pkg/translator/prometheusremotewrite/metrics_to_prw.go has a method FromMetrics():

// FromMetrics converts pmetric.Metrics to prometheus remote write format.
func FromMetrics(md pmetric.Metrics, settings Settings) (tsMap map[string]*prompb.TimeSeries, errs error) {
    ...
    // check for valid type and temporality combination and for matching data field and type
    if ok := validateMetrics(metric); !ok {
	    errs = multierr.Append(errs, errors.New("invalid temporality and type combination"))
	    continue
    }
    ...
}

Due to the fact that an invalid metric comes to the opentelemetry-collector-contrib (in my case, SUM with len(DataPoints) == 0), I get error "invalid temporality and type combination" when exporting data into prometheusremotewrite from opentelemetry-collector-contrib.

// validateMetrics returns a bool representing whether the metric has a valid type and temporality combination and a
// matching metric type and field
func validateMetrics(metric pmetric.Metric) bool {
	switch metric.Type() {
	case pmetric.MetricTypeGauge:
		return metric.Gauge().DataPoints().Len() != 0
	case pmetric.MetricTypeSum:
		return metric.Sum().DataPoints().Len() != 0 && metric.Sum().AggregationTemporality() == pmetric.AggregationTemporalityCumulative
	case pmetric.MetricTypeHistogram:
		return metric.Histogram().DataPoints().Len() != 0 && metric.Histogram().AggregationTemporality() == pmetric.AggregationTemporalityCumulative
	case pmetric.MetricTypeSummary:
		return metric.Summary().DataPoints().Len() != 0
	}
	return false
}

Before version 0.32.0 we had at least one DataPoint.

func sumPoint(record export.Record, num number.Number, start, end time.Time, temporality aggregation.Temporality, monotonic bool) (*metricpb.Metric, error) {}

After 0.32.0 we have zero or more DataPoint into grpc request to opentelemetry-collector.

Environment

  • OS: Linux
  • Architecture: x86
  • Go Version: 1.18
  • opentelemetry-go version: latest

Steps To Reproduce

  1. Create counter metric with name, example, accept_request
  2. Don't increase the counter
  3. Export the counter to opentelemetry-collector-contib. It will happen automatically.
  4. Export the counter by prometheusremotewrite method (example to victoria metrics) from opentelemetry-collector-contib. It will happen automatically.
  5. We get error "invalid temporality and type combination".

Expected behavior

On the one hand, I expect this error to not occur.

On the other hand, I want the metric with the current value to be sent to opentelemetry-collector-contrib
and then to victoria metrics by prometheusremotewrite method.

@krupyansky krupyansky added the bug Something isn't working label Oct 25, 2022
@MrAlias
Copy link
Contributor

MrAlias commented Oct 27, 2022

Can you clarify your "steps to reproduce" with code?

I'm very confused by this issue. Are you wondering why a counter that you created with this repository, https://github.com/open-telemetry/opentelemetry-go, doesn't export any data if you don't record any values?

@MrAlias MrAlias added response needed Waiting on user input before progress can be made area:metrics Part of OpenTelemetry Metrics labels Oct 27, 2022
@krupyansky
Copy link
Author

https://github.com/krupyansky/otel-bug

  1. make init
  2. make curl-metrics
  3. wait one-two minutes
  4. make logs-otel
  5. and you can see "Exporting failed. The error is not retryable. Dropping data. {"kind": "exporter", "data_type": "metrics", "name": "prometheusremotewrite", "error": "Permanent error: Permanent error: Post "victoriametrics:8428": unsupported protocol scheme "victoriametrics"", "dropped_items": 2}"

@MrAlias
Copy link
Contributor

MrAlias commented Oct 30, 2022

"Permanent error: Permanent error: Post "victoriametrics:8428": unsupported protocol scheme "victoriametrics""

It looks like you have an "unsupported protocol scheme" in your collector configuration. I would recommend correcting your configured endpoint.

@krupyansky
Copy link
Author

Yes, thanks for the note @MrAlias

Made changes in https://github.com/krupyansky/otel-bug.

Steps:

  1. make init
  2. make curl-metrics
  3. wait one-two minutes
  4. make logs-otel
  5. and you can see "Exporting failed. The error is not retryable. Dropping data. {"kind": "exporter", "data_type": "metrics", "name": "prometheusremotewrite", "error": "Permanent error: invalid temporality and type combination", "dropped_items": 2}"

@MrAlias
Copy link
Contributor

MrAlias commented Nov 1, 2022

If we returned nil here:

Instead of a value, then this:

data := inst.aggregator.Aggregation()
if data != nil {

Would drop the empty aggregation and the data would not be sent.

I think this is appropriate. I was considering if this current behavior is desirable because it signals the instrument exists and no measurements have been made. But that is not important. It is only important to signal that existing measurements already made have had no increment.

@MrAlias MrAlias removed the response needed Waiting on user input before progress can be made label Nov 2, 2022
@MrAlias MrAlias moved this to In Progress in Go: Metric SDK (Beta) Nov 2, 2022
@MrAlias MrAlias self-assigned this Nov 2, 2022
@MrAlias MrAlias added this to the Metric v0.34.0 milestone Nov 2, 2022
Repository owner moved this from In Progress to Done in Go: Metric SDK (Beta) Nov 11, 2022
wadexu007 added a commit to wadexu007/learning_by_doing that referenced this issue Dec 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:metrics Part of OpenTelemetry Metrics bug Something isn't working
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants