-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
otlpreceiver: Add Log Support #1444
otlpreceiver: Add Log Support #1444
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1444 +/- ##
==========================================
+ Coverage 90.70% 90.75% +0.05%
==========================================
Files 232 233 +1
Lines 16384 16423 +39
==========================================
+ Hits 14861 14905 +44
+ Misses 1092 1086 -6
- Partials 431 432 +1
Continue to review full report at Codecov.
|
Did not plan to do a full review, but saw that and want to point it out :) |
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.
LGTM
require.NoError(t, err, "Failed to export trace: %v", err) | ||
require.NotNil(t, resp, "The response is missing") | ||
|
||
// assert |
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.
What is this comment about?
_, port, doneFn := otlpReceiverOnGRPCServer(t, logSink) | ||
defer doneFn() | ||
|
||
traceClient, traceClientDoneFn, err := makeLogsServiceClient(port) |
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.
logsClient?
require.NoError(t, err, "Failed to create the TraceServiceClient: %v", err) | ||
defer traceClientDoneFn() | ||
|
||
// when |
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.
Unnecessary comment?
@bogdandrutu sorry, didn't notice that you had pending comments on this. |
* add OpenCensus metric exporter bridge * Update bridge/opencensus/README.md Co-authored-by: Eric Sirianni <[email protected]> Co-authored-by: Eric Sirianni <[email protected]> Co-authored-by: Tyler Yahn <[email protected]>
No description provided.