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

A way to export internal data as OTLP proto format #1719

Closed
chenzhihao opened this issue Sep 2, 2020 · 11 comments
Closed

A way to export internal data as OTLP proto format #1719

chenzhihao opened this issue Sep 2, 2020 · 11 comments
Labels
enhancement New feature or request
Milestone

Comments

@chenzhihao
Copy link
Contributor

Is your feature request related to a problem? Please describe.

We are trying to add a customised OPTL collector Exporter to send the metrics and spans via HTTP1.1 to our ingestion service by using OPTL protobuf binary as the transport data format.

But we can't simply convert the pdata.Traces and pdata.Metrics to the OPTL Protobuf-based structs as these structs are in the private fields of pdata in the implementation of OPTL collector:

type Traces struct {
	orig *[]*otlptrace.ResourceSpans
}
type MetricData struct {
	orig *[]*otlpmetrics.ResourceMetrics
}

Describe the solution you'd like

Create an HTTP equivalent of opentelemetry-collector/exporter/otlpexporter and opentelemetry-collector/exporter/otlpexporter which export the spans and metrics via HTTP1.1 by using OPTL protobuf binary as the transport data format.

Describe alternatives you've considered

Add public APIs in OPTL collector which can map the internal data to OPTL protobuf-based structs.
e.g. a public helper function as func MetricDataToOtlp(m pdata.Metrics) *otlpmetrics.ResourceMetrics {}

Additional context

We considered to follow the implementation of opentelemtry-collector-contribute to map the pdata to OpenCensus Protobuf-based structs:

https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/master/exporter/newrelicexporter/newrelic.go#L86

https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/master/exporter/newrelicexporter/newrelic.go#L117

But we thought this is not an ideal long term solution as eventually OpenCensus could be retired.

@bogdandrutu
Copy link
Member

Solving partially this #1720

@chenzhihao
Copy link
Contributor Author

chenzhihao commented Sep 2, 2020

@bogdandrutu Thanks for your reply!

After applying this PR we can map the internal data to []*otlpmetrics.ResourceMetrics, but the metric batch struct ExportMetricsServiceRequest is still private and can not be used by us directly: https://github.com/open-telemetry/opentelemetry-collector/blob/master/internal/data/opentelemetry-proto-gen/collector/metrics/v1/metrics_service.pb.go#L38

Could we ask you to shed some lights on how should we use the generated OPTL protobuf structs? Does that make sense to either:

  • move the /internal/data to pkg/data so that we can use these protobuf structs to build the customised Collector exporter
  • or use a central public repo for the generated protobuf like what OpenCesus did
  • or have some public helper functions to generate the ExportMetricsServiceRequest struct

Also, will we apply a similar change for span soon?

@bogdandrutu
Copy link
Member

There are 2 things:

  1. Soon will have time to implement the otlp over http.
  2. We still don't know how and if we want to expose the proto that we use internally (we may reimplement some messages for performance, etc). So looks like a decision in that sense is required as well.

Probably this week I will give you the exporter, then will spend more time thinking about the protos, as I said is not just exposing is also making sure that we don't stop our progress to improve performance

@bogdandrutu
Copy link
Member

It may be easier to expose a ToProtoBytes method that serializes things for you as well as a ToJsonString without exposing the data structures that we use to encode/decode

@tigrannajaryan tigrannajaryan added this to the Backlog milestone Sep 2, 2020
@tigrannajaryan
Copy link
Member

@chenzhihao any reason you do not want to use official OTLP/HTTP protocol?

@bogdandrutu
Copy link
Member

I can see one: It is not implemented yet :)

@tigrannajaryan
Copy link
Member

For reference, OTLP/HTTP exporter is planned for GA: #882 and we could use some help to implement it.

@tigrannajaryan tigrannajaryan changed the title A way to export internal data as OPTL proto format A way to export internal data as OTLP proto format Sep 2, 2020
@chenzhihao
Copy link
Contributor Author

It may be easier to expose a ToProtoBytes method that serializes things for you as well as a ToJsonString without exposing the data structures that we use to encode/decode

@bogdandrutu if we could have the helper method ToProtoBytes to generate the marshalled ExportMetricsServiceRequest, ExportTraceServiceRequest, that would be really helpful!

(If we could eventually have the exporter which can transport otlp over http that would be awesome!)

@bogdandrutu
Copy link
Member

The exporter is coming soon. The helper methods feel free to craft a PR, I need to see how it looks before making a decision (just the signatures is enough)

@chenzhihao
Copy link
Contributor Author

chenzhihao commented Sep 3, 2020

@bogdandrutu I created a PR draft for the functions: #1741

Do you think this makes sense? If we can make a decision I will go on working on it.

@chenzhihao
Copy link
Contributor Author

I think this may give us more flexibilities for consuming these Protobuf bytes comparing to using the coming OTLP/HTTP exporter directly.

For instance, we are aiming to send these protobuf bytes via HTTP1.1 with some customized HTTP header metadata. This kind of specific requirement could be hard to be achieved by using the coming OTLP/HTTP exporter.

tigrannajaryan pushed a commit that referenced this issue Sep 17, 2020
…uf bytes (#1741)

Add the public functions to export internal pipeline data to otlp `ExportLogsServiceRequest`, `ExportMetricsServiceRequest`, `ExportTraceServiceRequest`.

The intention for this feature is to add a way to export the internal data to protbuf bytes without exposing the internal protobuf generated data structure. Thus the user can send this otlp data over HTTP.


**Link to tracking Issue:** 
#1719

**Testing:**
Unit test added.
@andrewhsu andrewhsu added the enhancement New feature or request label Jan 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants