Exporter: Initial Jaeger Trace Exporter commit. #112
Conversation
rghetia
requested review from
bogdandrutu,
jmacd,
pjanotti,
sjkaris and
tedsuo
as code owners
exporter/trace/jaeger/jaeger.go
Outdated
| } | ||
|
|
||
| // BoolTag creates a new tag of type bool, exported as jaeger.TagType_BOOL | ||
| func BoolTag(key string, value bool) Tag { |
There was a problem hiding this comment.
Looks like this and the other Tag constructors here are not used? I see attributeToTag might have once used them? Can we remove this Tag?
| } | ||
| } | ||
|
|
||
| func attributeToTag(key string, a interface{}) *gen.Tag { |
There was a problem hiding this comment.
Seems like we could avoid the interface{} conversation here by passing through the underlying value, i.e., not Emitting it yet. This would avoid some allocation. Fine to leave a TODO.
There was a problem hiding this comment.
added a TODO for now.
| return nil | ||
| } | ||
|
|
||
| func serialize(obj thrift.TStruct) (*bytes.Buffer, error) { |
There was a problem hiding this comment.
I'm getting confused about the (?) different modes of output. Is there both a UDP sender and a HTTP sender? I was wondering when a thrift memory buffer would be re-used, vs. allocated for every send. Can you comment?
There was a problem hiding this comment.
There are two methods to export.
- Via Agent - It uses UDP to send data to the agent.
- Directly to Collector - It uses HTTP. (It could use GRPC).
It will only use one or the other. Second method is a preferred method.
I am not familiar with Thrift as much. I can look into it to check if there is a possibility of some optimization wrt memory buffer allocation. For now is it ok to put a TODO here?
There was a problem hiding this comment.
OK, that helps. I don't think a TODO is needed, I just wanted to be sure I knew what was happening.
There was a problem hiding this comment.
Probably too late but, I was wondering why not to use the THTTPClient(https://github.com/apache/thrift/blob/master/lib/go/thrift/http_client.go) instead of having an intermediate TMemoryBuffer.
For the reference: open-telemetry/opentelemetry-python#508 (comment).
rghetia
force-pushed
the
exporter_jaeger
branch
from
71840ec to
bbe30a5
Compare
|
@jmacd I had to export Event struct to fix the build. If there are any comments associated with this change then I will it in another PR. For now I am merging this. |
* prepare for release_v0.7.0 * Upgrade metric exporter code to otel-v0.7.0 API * Remove internal trace standard helpers in favor of api/standard from otel-go * fixup go.mod Co-authored-by: Evan Torrie <evantorrie@users.noreply.github.com> Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
fixes #95