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.

Sign up for GitHub

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

Jump to bottom

[WIP][collector] Switch to OTEL's http/grpc server #6277

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

[WIP][collector] Switch to OTEL's http/grpc server #6277

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
641ddf2
use otel helper in collector
chahatsagarmain Nov 29, 2024
9bb39e2
add server config instead of options
chahatsagarmain Nov 29, 2024
4bf8eaa
migrate to otel config
chahatsagarmain Nov 29, 2024
31588ef
deletion of unused functions
chahatsagarmain Nov 29, 2024
624bb69
tenancy and fixes
chahatsagarmain Nov 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 2 additions & 18 deletions cmd/collector/app/handler/otlp_receiver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"go.opentelemetry.io/collector/config/configgrpc"
"go.opentelemetry.io/collector/config/confighttp"
"go.opentelemetry.io/collector/config/configtelemetry"
"go.opentelemetry.io/collector/config/configtls"
"go.opentelemetry.io/collector/consumer"
"go.opentelemetry.io/collector/extension"
"go.opentelemetry.io/collector/pdata/ptrace"
Expand All @@ -27,7 +26,6 @@ import (

"github.com/jaegertracing/jaeger/cmd/collector/app/flags"
"github.com/jaegertracing/jaeger/cmd/collector/app/processor"
"github.com/jaegertracing/jaeger/pkg/config/tlscfg"
"github.com/jaegertracing/jaeger/pkg/tenancy"
)

Expand Down Expand Up @@ -106,7 +104,7 @@ func applyGRPCSettings(cfg *configgrpc.ServerConfig, opts *flags.GRPCOptions) {
cfg.NetAddr.Endpoint = opts.HostPort
}
if opts.TLS.Enabled {
cfg.TLSSetting = applyTLSSettings(&opts.TLS)
cfg.TLSSetting = opts.TLS.ToOtelServerConfig()
Copy link
Member

Choose a reason for hiding this comment

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

the approach in the other PRs is to replace the config field type with configtls, can we use that here too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we use configtls.ServerConfig here

jaeger/cmd/collector/app/flags/flags.go

Line 145 in 641ddf2

TLS tlscfg.Options
but the config has no enabled field or insecure field like clientConfig . Then how should i deal with opts.TLS.Enabled

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ToOtelServerConfig just returns nil if tls is not enabled , This will work .

Copy link
Member

Choose a reason for hiding this comment

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

Then how should i deal with opts.TLS.Enabled

we should deal with it the same way as in the other PRs. The interface in v1 is CLI flags, it doesn't matter if we translate them into internal TLS config of the one from OTEL.

}
if opts.MaxReceiveMessageLength > 0 {
cfg.MaxRecvMsgSizeMiB = int(opts.MaxReceiveMessageLength / (1024 * 1024))
Expand All @@ -126,7 +124,7 @@ func applyHTTPSettings(cfg *confighttp.ServerConfig, opts *flags.HTTPOptions) {
cfg.Endpoint = opts.HostPort
}
if opts.TLS.Enabled {
cfg.TLSSetting = applyTLSSettings(&opts.TLS)
cfg.TLSSetting = opts.TLS.ToOtelServerConfig()
}

cfg.CORS = &confighttp.CORSConfig{
Expand All @@ -135,20 +133,6 @@ func applyHTTPSettings(cfg *confighttp.ServerConfig, opts *flags.HTTPOptions) {
}
}

func applyTLSSettings(opts *tlscfg.Options) *configtls.ServerConfig {
return &configtls.ServerConfig{
Config: configtls.Config{
CAFile: opts.CAPath,
CertFile: opts.CertPath,
KeyFile: opts.KeyPath,
MinVersion: opts.MinVersion,
MaxVersion: opts.MaxVersion,
ReloadInterval: opts.ReloadInterval,
},
ClientCAFile: opts.ClientCAPath,
}
}

func newConsumerDelegate(logger *zap.Logger, spanProcessor processor.SpanProcessor, tm *tenancy.Manager) *consumerDelegate {
return &consumerDelegate{
batchConsumer: newBatchConsumer(logger,
Expand Down
5 changes: 1 addition & 4 deletions cmd/collector/app/handler/zipkin_receiver_tls_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
"time"

"github.com/stretchr/testify/require"
"go.uber.org/zap"

"github.com/jaegertracing/jaeger/cmd/collector/app/flags"
"github.com/jaegertracing/jaeger/pkg/config/tlscfg"
Expand Down Expand Up @@ -165,7 +164,6 @@ func TestSpanCollectorZipkinTLS(t *testing.T) {
opts := &flags.CollectorOptions{}
opts.Zipkin.HTTPHostPort = ports.PortToHostPort(ports.CollectorZipkin)
opts.Zipkin.TLS = test.serverTLS
defer test.serverTLS.Close()

server, err := StartZipkinReceiver(opts, logger, spanProcessor, tm)
if test.expectServerFail {
Expand All @@ -177,8 +175,7 @@ func TestSpanCollectorZipkinTLS(t *testing.T) {
require.NoError(t, server.Shutdown(context.Background()))
}()

clientTLSCfg, err0 := test.clientTLS.Config(zap.NewNop())
defer test.clientTLS.Close()
clientTLSCfg, err0 := test.clientTLS.ToOtelClientConfig().LoadTLSConfig(context.Background())
require.NoError(t, err0)
dialer := &net.Dialer{Timeout: 2 * time.Second}
conn, clientError := tls.DialWithDialer(dialer, "tcp", fmt.Sprintf("localhost:%d", ports.CollectorZipkin), clientTLSCfg)
Expand Down
3 changes: 2 additions & 1 deletion cmd/collector/app/server/grpc.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package server

import (
"context"
"fmt"
"net"
"time"
Expand Down Expand Up @@ -54,7 +55,7 @@ func StartGRPCServer(params *GRPCServerParams) (*grpc.Server, error) {

if params.TLSConfig.Enabled {
// user requested a server with TLS, setup creds
tlsCfg, err := params.TLSConfig.Config(params.Logger)
tlsCfg, err := params.TLSConfig.ToOtelServerConfig().LoadTLSConfig(context.Background())
if err != nil {
return nil, err
}
Expand Down
1 change: 0 additions & 1 deletion cmd/collector/app/server/grpc_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ func TestCollectorStartWithTLS(t *testing.T) {
server, err := StartGRPCServer(params)
require.NoError(t, err)
defer server.Stop()
defer params.TLSConfig.Close()
}

func TestCollectorReflection(t *testing.T) {
Expand Down
3 changes: 2 additions & 1 deletion cmd/collector/app/server/http.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package server

import (
"context"
"net"
"net/http"
"time"
Expand Down Expand Up @@ -53,7 +54,7 @@ func StartHTTPServer(params *HTTPServerParams) (*http.Server, error) {
ErrorLog: errorLog,
}
if params.TLSConfig.Enabled {
yurishkuro marked this conversation as resolved.
Show resolved Hide resolved
tlsCfg, err := params.TLSConfig.Config(params.Logger) // This checks if the certificates are correctly provided
tlsCfg, err := params.TLSConfig.ToOtelServerConfig().LoadTLSConfig(context.Background())
if err != nil {
return nil, err
}
Expand Down
6 changes: 2 additions & 4 deletions cmd/collector/app/server/http_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package server

import (
"context"
"crypto/tls"
"fmt"
"net"
Expand Down Expand Up @@ -54,7 +55,6 @@ func TestCreateTLSHTTPServerError(t *testing.T) {
}
_, err := StartHTTPServer(params)
require.Error(t, err)
defer params.TLSConfig.Close()
}

func TestSpanCollectorHTTP(t *testing.T) {
Expand Down Expand Up @@ -196,16 +196,14 @@ func TestSpanCollectorHTTPS(t *testing.T) {
Logger: logger,
TLSConfig: test.TLS,
}
defer params.TLSConfig.Close()

server, err := StartHTTPServer(params)
require.NoError(t, err)
defer func() {
require.NoError(t, server.Close())
}()

clientTLSCfg, err0 := test.clientTLS.Config(logger)
defer test.clientTLS.Close()
clientTLSCfg, err0 := test.clientTLS.ToOtelClientConfig().LoadTLSConfig(context.Background())
require.NoError(t, err0)
dialer := &net.Dialer{Timeout: 2 * time.Second}
conn, clientError := tls.DialWithDialer(dialer, "tcp", "localhost:"+strconv.Itoa(ports.CollectorHTTP), clientTLSCfg)
Expand Down
Loading