-
Notifications
You must be signed in to change notification settings - Fork 248
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
OTLP exporter #277
OTLP exporter #277
Conversation
There are two open issues:
Otherwise this is ready for review. |
@ericmustin I'd appreciate your 👀 on this, since you have some experience implementing exporters. 🙏 |
end | ||
|
||
it 'integrates with collector' do | ||
skip unless ENV['TRACING_INTEGRATION_TEST'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've run this locally with:
TRACING_INTEGRATION_TEST=1 bundle exec rake test
and the collector:
./otelcol --config logging.yml
where logging.yml
is:
receivers:
otlp:
protocols:
http:
exporters:
logging:
extensions:
zpages:
service:
extensions: [zpages]
pipelines:
traces:
receivers: [otlp]
exporters: [logging]
It produces the output:
2020-07-29T22:05:48.751-0400 INFO loggingexporter/logging_exporter.go:102 TraceExporter {"#spans": 1}
And metrics:
Franciss-MacBook-Pro:otlp bogsanyi$ curl http://localhost:8888/metrics
# HELP otelcol_exporter_send_failed_spans Number of spans in failed attempts to send to destination.
# TYPE otelcol_exporter_send_failed_spans counter
otelcol_exporter_send_failed_spans{exporter="logging",service_instance_id="d5ec4873-08ce-4142-b8ea-c282714a95be"} 0
# HELP otelcol_exporter_sent_spans Number of spans successfully sent to destination.
# TYPE otelcol_exporter_sent_spans counter
otelcol_exporter_sent_spans{exporter="logging",service_instance_id="d5ec4873-08ce-4142-b8ea-c282714a95be"} 1
# HELP otelcol_process_runtime_heap_alloc_bytes Bytes of allocated heap objects (see 'go doc runtime.MemStats.HeapAlloc')
# TYPE otelcol_process_runtime_heap_alloc_bytes gauge
otelcol_process_runtime_heap_alloc_bytes{service_instance_id="d5ec4873-08ce-4142-b8ea-c282714a95be"} 4.49036e+06
# HELP otelcol_process_runtime_total_alloc_bytes Cumulative bytes allocated for heap objects (see 'go doc runtime.MemStats.TotalAlloc')
# TYPE otelcol_process_runtime_total_alloc_bytes gauge
otelcol_process_runtime_total_alloc_bytes{service_instance_id="d5ec4873-08ce-4142-b8ea-c282714a95be"} 9.477112e+06
# HELP otelcol_process_runtime_total_sys_memory_bytes Total bytes of memory obtained from the OS (see 'go doc runtime.MemStats.Sys')
# TYPE otelcol_process_runtime_total_sys_memory_bytes gauge
otelcol_process_runtime_total_sys_memory_bytes{service_instance_id="d5ec4873-08ce-4142-b8ea-c282714a95be"} 7.2286456e+07
# HELP otelcol_receiver_accepted_spans Number of spans successfully pushed into the pipeline.
# TYPE otelcol_receiver_accepted_spans counter
otelcol_receiver_accepted_spans{receiver="otlp",service_instance_id="d5ec4873-08ce-4142-b8ea-c282714a95be",transport="grpc"} 1
# HELP otelcol_receiver_refused_spans Number of spans that could not be pushed into the pipeline.
# TYPE otelcol_receiver_refused_spans counter
otelcol_receiver_refused_spans{receiver="otlp",service_instance_id="d5ec4873-08ce-4142-b8ea-c282714a95be",transport="grpc"} 0
@fbogsany yup happy to look, should be able to block off some time in the coming days...one thing that jumps out is it isn't immediately clear to me how |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left some small comments but there's nothing blocking for me. Not that my approval is needed here, but as there are still a few TODOs you have mentioned I've held off on giving an approval until those are implemented, but overall this LGTM and am happy to approve if you want to kick the TODOs to a later PR. Nice work on this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, LGTM. I have couple of comments though.
I've rebased this and made some minor fixes for API changes. I haven't addressed feedback yet, or the open issues. |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a few small questions about making sure we safely encode spans that may not have resource and instrumentation_library, and left suggestions about the custom header support, but i'm fine with any approach here, and if we can't reach consensus i'm also fine pushing this feature to a later PR/release. There also look to be one or two ENV Var tests that are still marked as TODOs, if you want to punt on those to a later PR i think that's completely fine as well.
I think this is very close imo, the only real blocker to me is that jruby appears to be having some dependancy issues with the added gems.
a06c496
to
e8ac9af
Compare
I think I've done everything I said I would, except for:
I honestly lack the energy to do this. I'd rather merge what we have here and take some time afterwards to talk through how to handle the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather merge what we have here and take some time afterwards to talk through how to handle the OTEL_EXPORTER_OTLP_PROTOCOL config option. I'm less convinced now that the gem rename is the right approach (but I also don't know what the "right thing" is).
Concur, let's not let perfect be the enemy of good as this is very robust.
This adds a gem containing an exporter for OTLP over HTTP (trace only).
To do:
GZIP(Requires upstream support)Handling redirects(Punting to a subsequent PR)span.resource
opentelemetry-exporter-otlp-http
(gRPC support will be added later asopentelemetry-exporter-otlp-grpc
)Blockers
master
protobuf definitions, including Add support for arrays and maps for attribute values opentelemetry-proto#157. The collector does not yet support this version, so our end-to-end tests will be blocked until the collector receiver is updated.