Add OC to OTLP translation functions #546
Conversation
tigrannajaryan
requested review from
bogdandrutu,
flands,
owais,
pjanotti,
rghetia and
songy23
as code owners
tigrannajaryan
force-pushed
the
feature/tigran/translators
branch
from
ff85dd3 to
ad4ffba
Compare
Codecov Report
@@ Coverage Diff @@
## master #546 +/- ##
==========================================
+ Coverage 76.19% 76.33% +0.14%
==========================================
Files 129 130 +1
Lines 7993 8228 +235
==========================================
+ Hits 6090 6281 +191
- Misses 1602 1643 +41
- Partials 301 304 +3
Continue to review full report at Codecov.
|
| @@ -41,13 +41,17 @@ type TraceData struct { | |||
| // OTLPTraceData is a struct that groups proto spans with a resource. This is the | |||
| // newer version of TraceData, using OTLP-based representation. | |||
| type OTLPTraceData struct { | |||
| ResourceSpanList []*otlptrace.ResourceSpans | |||
| resourceSpanList []*otlptrace.ResourceSpans | |||
There was a problem hiding this comment.
Are we making this private temporarily to prevent usage before this is ready?
There was a problem hiding this comment.
That's one of the reasons. The other is that I may keep it private and require certain modifications to go through functions. It will become clearer in future PRs. Let's keep it private for now.
| return | ||
| } | ||
| return time.Unix(ts.Seconds, int64(ts.Nanos)) | ||
| } |
There was a problem hiding this comment.
This is quite useful in many places. I think we should have a package to deal with timestamps in general as I've seen this and very similar function scattered across packages in this and contrib repo.
There was a problem hiding this comment.
I moved this from Jaeger translation code. This can be now reused everywhere.
There was a problem hiding this comment.
Later we should move it out of internal so contrib can also use it, a translation package seems a good candidate.
translator/trace/oc_to_otlp.go
Outdated
| DroppedEventsCount: droppedEventCount, | ||
| Links: links, | ||
| DroppedLinksCount: droppedLinkCount, | ||
| Status: nil, |
There was a problem hiding this comment.
No, thanks for catching! Completely missed the Status translation.
There was a problem hiding this comment.
Added, please have a look.
tigrannajaryan
force-pushed
the
feature/tigran/translators
branch
2 times, most recently
from
7b1c6ed to
257516a
Compare
| @@ -47,8 +47,8 @@ func ProtoSpanToOCSpanData(span *tracepb.Span) (*trace.SpanData, error) { | |||
| sd := &trace.SpanData{ | |||
| SpanContext: sc, | |||
| ParentSpanID: parentSpanID, | |||
| StartTime: timestampToTime(span.StartTime), | |||
| EndTime: timestampToTime(span.EndTime), | |||
| StartTime: internal.TimestampToTime(span.StartTime), | |||
There was a problem hiding this comment.
I missed this earlier but internal is not a good name for a package.
There was a problem hiding this comment.
This is not a new package. We can rename it to something better in an future PR.
tigrannajaryan
force-pushed
the
feature/tigran/translators
branch
from
257516a to
8681bb8
Compare
|
@pjanotti please have another look. |
tigrannajaryan
force-pushed
the
feature/tigran/translators
branch
from
8681bb8 to
10ad229
Compare
tigrannajaryan
force-pushed
the
feature/tigran/translators
branch
from
10ad229 to
d6c6e25
Compare
This implements OC to OTLP protobuf message translation. The code is currently reachable only from the tests. We probably need a bit more coverage for this code but I am submitting this change as is for the sake of keeping the size of the PR managable for reviewing. Contributes to open-telemetry#478
tigrannajaryan
force-pushed
the
feature/tigran/translators
branch
from
d6c6e25 to
4f87c73
Compare
* Add MinMaxSumCount stress test * Reimplement MinMaxSumCount using StateLocker * Address PR comments * Round open-telemetry#2 of PR comments Co-authored-by: Rahul Patel <rahulpa@google.com>
Believe this is called metricstransform (not metrictransform)
- Adds 3 new components to the demo as part of the 1.2 release - accountingservice - fraudetectionservice - kafka - necessary changes to checkoutservice were also done. - adds support for frontendproxy to emit traces (new feature in demo 1.2 release) - use ghcr images instead of docker hub ones - Some re-ordering of the components took place to better organize them based on key components (alpha sorted), followed by supplemental components (alpha sorted)
This implements OC to OTLP protobuf message translation.
The code is currently reachable only from the tests.
We probably need a bit more coverage for this code but I am
submitting this change as is for the sake of keeping the
size of the PR managable for reviewing.
Contributes to #478