opentelemetry collector trace exporter #497
Conversation
rghetia
requested review from
iredelmeier,
jmacd,
krnowak,
lizthegrey,
paivagustavo and
tedsuo
as code owners
| out = append(out, &commonpb.AttributeKeyValue{ | ||
| Key: string(v.Key), | ||
| Type: commonpb.AttributeKeyValue_INT, | ||
| IntValue: v.Value.AsInt64(), |
There was a problem hiding this comment.
I don't really know if this isn't going to trip us in some way if type is, for example, core.INT32. The value contains uint64 containing int32 and we are returning it as uint64 without any cast. This might work by accident.
Just a note: AsInt64 does not do any coercion. It interprets the bits in value as integral 64-bit value.
There was a problem hiding this comment.
This looks OK to me, because when the Value is constructed for a int32 value, we call
func int32ToRaw(i int32) uint64 {
return uint64(i)
}
exporter/trace/otelcol/README.md
Outdated
| @@ -0,0 +1,24 @@ | |||
| # OpenTelemetry Collector Go Exporter | |||
There was a problem hiding this comment.
Did you consider naming this "otlp"? It seems like the term we've used for this protocol everywhere.
There was a problem hiding this comment.
I'll change this to otlp inline with receiver in opentelemetry-collector.
| out = append(out, &commonpb.AttributeKeyValue{ | ||
| Key: string(v.Key), | ||
| Type: commonpb.AttributeKeyValue_INT, | ||
| IntValue: v.Value.AsInt64(), |
There was a problem hiding this comment.
This looks OK to me, because when the Value is constructed for a int32 value, we call
func int32ToRaw(i int32) uint64 {
return uint64(i)
}
|
I've been prototyping the metrics version of this exporter (#377) and realize there is going to be a fair amount of repeated code for that Exporter (setup/teardown to the collector, configuration, value parsing, ...). I'm wondering, given this will be the first exporter included in the project that will export both Spans and Metrics, if we want to locate this somewhere else. I think if we moved all exporters up one level, removed the If that option doesn't sit well, we could also possibly move this only to in the |
|
Moving one level up is fine. If there is a need for a separate exporter package then we can always create |
rghetia
force-pushed
the
opentelemetry-exporter
branch
from
f7c392e
to
a10e2c4
Compare
|
@MrAlias if you don't mind can you please review this? |
Fixed #12 - [x] Bootstrap opentelemetry - [x] By default export json - [x] Root trace is the cmd.Execute - [x] One span for every goroutine (measure concurrency) * - [ ] Allow to export to collector (blocked by open-telemetry/opentelemetry-go#497) - [ ] Find a way to trace kubernetes client-go - [ ] Trace http.Client that makes requests to profefe API * In order to get the gathering of the profile efficient I use a channel that spread the work across many goroutine (atm fixed to 10). The root span has a child for any goroutine. Signed-off-by: Gianluca Arbezzano <gianarb92@gmail.com>
- missing load test - missing resources
rghetia
force-pushed
the
opentelemetry-exporter
branch
from
241940a
to
e6adf8f
Compare
* opentelemetry collector exporter - missing load test - missing resources * fix review comments. * add test for each SpanKind and Attribute Type. * rename otelcol to otlp * move exporter/trace/otlp to exporters/otlp * more review comments. * add alignment test. * pass context to uploadSpans
fixes #376
This is largely taken from ocagent exporter.
Few items are still missing
test-386 is skipped for Mac OS 10.15.x (Catalina and upwards).