-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Add OTLP receiver to collector #3701
Conversation
Codecov Report
@@ Coverage Diff @@
## main #3701 +/- ##
==========================================
+ Coverage 97.34% 97.44% +0.10%
==========================================
Files 268 269 +1
Lines 15752 15842 +90
==========================================
+ Hits 15333 15438 +105
+ Misses 330 320 -10
+ Partials 89 84 -5
Continue to review full report at Codecov.
|
1f7a70c
to
8ae69d1
Compare
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
@albertteoh care to review? |
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 overall. Added some non-blocking suggestions .
logger.Fatal("Failed to start collector", zap.Error(err)) | ||
} | ||
|
||
// Wait for shutfown |
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.
// Wait for shutfown | |
// Wait for shutdown |
expectErr error | ||
expectLog string | ||
}{ | ||
{}, // no errors |
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 would add a test name
or description
so there won't be a need for this comment. i.e.:
{name: "no errors"},
...
t.Run(test.name, func(t *testing.T) {...
It has the added benefit of a more descriptive test name in logs when executing the test.
Signed-off-by: Yuri Shkuro <[email protected]> Co-authored-by: Albert <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
@@ -84,6 +84,11 @@ type CollectorOptions struct { | |||
// See gRPC's keepalive.ServerParameters#MaxConnectionAgeGrace. | |||
MaxConnectionAgeGrace time.Duration | |||
} | |||
// OTLP section defines options for servers accepting OpenTelemetry OTLP format | |||
OTLP struct { |
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.
refactored in #3710
var _ component.Host = (*otelHost)(nil) // API check | ||
|
||
// OtelReceiverOptions allows configuration of the receiver. | ||
type OtelReceiverOptions struct { |
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.
removed in #3710
Signed-off-by: Yuri Shkuro <[email protected]>
thanks for the review @albertteoh |
Signed-off-by: Yuri Shkuro <[email protected]> Co-authored-by: Albert <[email protected]> Signed-off-by: Albert Teoh <[email protected]>
A proof-of-concept of adding an OTLP receiver (#3625) to Jaeger collector/all-in-one.
component.Host
)Testing with this little program https://gist.github.com/yurishkuro/c60011b9b3b3e6b5c11f0852f877a6c3 produces a trace like this: