Skip to content
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 OTEL config to all-in-one #1080

Merged
merged 4 commits into from
Jun 4, 2020

Conversation

pavolloffay
Copy link
Member

Signed-off-by: Pavol Loffay ploffay@redhat.com

Example CR

apiVersion: jaegertracing.io/v1
kind: Jaeger
metadata:
  name: simplest
spec:
  allInOne:
    image: jaegertracing/opentelemetry-all-in-one:latest
    config:
      extensions:
        health_check:
          port: 14269

Signed-off-by: Pavol Loffay <ploffay@redhat.com>
Signed-off-by: Pavol Loffay <ploffay@redhat.com>
@codecov
Copy link

codecov bot commented Jun 3, 2020

Codecov Report

Merging #1080 into master will increase coverage by 23.12%.
The diff coverage is 83.33%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master    #1080       +/-   ##
===========================================
+ Coverage   64.69%   87.81%   +23.12%     
===========================================
  Files          86       85        -1     
  Lines        7013     5172     -1841     
===========================================
+ Hits         4537     4542        +5     
+ Misses       2313      466     -1847     
- Partials      163      164        +1     
Impacted Files Coverage Δ
pkg/apis/jaegertracing/v1/jaeger_types.go 100.00% <ø> (ø)
pkg/config/otelconfig/otelconfig.go 81.39% <50.00%> (-0.96%) ⬇️
pkg/deployment/all_in_one.go 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c4418b6...3e5a50f. Read the comment docs.

@@ -60,21 +62,26 @@ func (a *AllInOne) Get() *appsv1.Deployment {
sampling.Update(a.jaeger, commonSpec, &options)
tls.Update(a.jaeger, commonSpec, &options)

if len(util.FindItem("--reporter.type=", options)) == 0 {
options = append(options, "--reporter.type=grpc")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not needed. there is only grpc reporter that is used by default

Signed-off-by: Pavol Loffay <ploffay@redhat.com>
@objectiser
Copy link
Contributor

Looks like coverage affected by error paths - possible add some invalid otel config in a separate test?

Signed-off-by: Pavol Loffay <ploffay@redhat.com>
@pavolloffay pavolloffay merged commit e48d7d3 into jaegertracing:master Jun 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants