Skip to content

Commit

Permalink
[query] Change HTTP and TLS server configurations to use OTEL configu…
Browse files Browse the repository at this point in the history
…rations (#6023)

## Which problem is this PR solving?
- Part of #5996

## Description of the changes
- Removed the custom TLS and HTTP server configurations and replaced
them with OTEL's configurations
- 🛑 Breaking change: the hot-reload of certificates will not happen at
`reload_interval`s rather than immediately on file changes. This makes
Jaeger's behavior similar to OTEL Collector's behavior.

## How was this change tested?
- Unit tests / CI

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [x] I have added unit tests for the new functionality
- [x] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

---------

Signed-off-by: Mahad Zaryab <mahadzaryab1@gmail.com>
  • Loading branch information
mahadzaryab1 authored Sep 28, 2024
1 parent 3cd1f59 commit 731f50e
Show file tree
Hide file tree
Showing 13 changed files with 445 additions and 284 deletions.
8 changes: 1 addition & 7 deletions cmd/jaeger/internal/extension/jaegerquery/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ package jaegerquery
import (
"github.com/asaskevich/govalidator"
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config/configgrpc"
"go.opentelemetry.io/collector/config/confighttp"

queryApp "github.com/jaegertracing/jaeger/cmd/query/app"
)
Expand All @@ -16,13 +14,9 @@ var _ component.ConfigValidator = (*Config)(nil)

// Config represents the configuration for jaeger-query,
type Config struct {
queryApp.QueryOptionsBase `mapstructure:",squash"`
queryApp.QueryOptions `mapstructure:",squash"`
// Storage holds configuration related to the various data stores that are to be queried.
Storage Storage `mapstructure:"storage"`
// HTTP holds the HTTP configuration that the query service uses to serve requests.
HTTP confighttp.ServerConfig `mapstructure:"http"`
// GRPC holds the GRPC configuration that the query service uses to serve requests.
GRPC configgrpc.ServerConfig `mapstructure:"grpc"`
}

type Storage struct {
Expand Down
17 changes: 10 additions & 7 deletions cmd/jaeger/internal/extension/jaegerquery/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"go.opentelemetry.io/collector/config/confignet"
"go.opentelemetry.io/collector/extension"

"github.com/jaegertracing/jaeger/cmd/query/app"
"github.com/jaegertracing/jaeger/ports"
)

Expand All @@ -27,13 +28,15 @@ func NewFactory() extension.Factory {

func createDefaultConfig() component.Config {
return &Config{
HTTP: confighttp.ServerConfig{
Endpoint: ports.PortToHostPort(ports.QueryHTTP),
},
GRPC: configgrpc.ServerConfig{
NetAddr: confignet.AddrConfig{
Endpoint: ports.PortToHostPort(ports.QueryGRPC),
Transport: confignet.TransportTypeTCP,
QueryOptions: app.QueryOptions{
HTTP: confighttp.ServerConfig{
Endpoint: ports.PortToHostPort(ports.QueryHTTP),
},
GRPC: configgrpc.ServerConfig{
NetAddr: confignet.AddrConfig{
Endpoint: ports.PortToHostPort(ports.QueryGRPC),
Transport: confignet.TransportTypeTCP,
},
},
},
}
Expand Down
13 changes: 1 addition & 12 deletions cmd/jaeger/internal/extension/jaegerquery/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func (s *server) Start(_ context.Context, host component.Host) error {
// TODO propagate healthcheck updates up to the collector's runtime
qs,
mqs,
s.makeQueryOptions(),
&s.config.QueryOptions,
tm,
telset,
)
Expand Down Expand Up @@ -158,17 +158,6 @@ func (s *server) createMetricReader(host component.Host) (metricsstore.Reader, e
return metricsReader, err
}

func (s *server) makeQueryOptions() *queryApp.QueryOptions {
return &queryApp.QueryOptions{
QueryOptionsBase: s.config.QueryOptionsBase,

// TODO utilize OTEL helpers for creating HTTP/GRPC servers
HTTPHostPort: s.config.HTTP.Endpoint,
GRPCHostPort: s.config.GRPC.NetAddr.Endpoint,
// TODO handle TLS
}
}

func (s *server) Shutdown(ctx context.Context) error {
var errs []error
if s.server != nil {
Expand Down
8 changes: 5 additions & 3 deletions cmd/query/app/additional_headers_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@ package app

import (
"net/http"

"go.opentelemetry.io/collector/config/configopaque"
)

func additionalHeadersHandler(h http.Handler, additionalHeaders http.Header) http.Handler {
func responseHeadersHandler(h http.Handler, responseHeaders map[string]configopaque.String) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
header := w.Header()
for key, values := range additionalHeaders {
header[key] = values
for key, value := range responseHeaders {
header.Set(key, value.String())
}

h.ServeHTTP(w, r)
Expand Down
19 changes: 10 additions & 9 deletions cmd/query/app/additional_headers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,21 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/config/configopaque"
)

func TestAdditionalHeadersHandler(t *testing.T) {
additionalHeaders := http.Header{}
additionalHeaders.Add("Access-Control-Allow-Origin", "https://mozilla.org")
additionalHeaders.Add("Access-Control-Expose-Headers", "X-My-Custom-Header")
additionalHeaders.Add("Access-Control-Expose-Headers", "X-Another-Custom-Header")
additionalHeaders.Add("Access-Control-Request-Headers", "field1, field2")
func TestResponseHeadersHandler(t *testing.T) {
responseHeaders := make(map[string]configopaque.String)
responseHeaders["Access-Control-Allow-Origin"] = "https://mozilla.org"
responseHeaders["Access-Control-Expose-Headers"] = "X-My-Custom-Header"
responseHeaders["Access-Control-Expose-Headers"] = "X-Another-Custom-Header"
responseHeaders["Access-Control-Request-Headers"] = "field1, field2"

emptyHandler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.Write([]byte{})
})

handler := additionalHeadersHandler(emptyHandler, additionalHeaders)
handler := responseHeadersHandler(emptyHandler, responseHeaders)
server := httptest.NewServer(handler)
defer server.Close()

Expand All @@ -33,7 +34,7 @@ func TestAdditionalHeadersHandler(t *testing.T) {
resp, err := server.Client().Do(req)
require.NoError(t, err)

for k, v := range additionalHeaders {
assert.Equal(t, v, resp.Header[k])
for k, v := range responseHeaders {
assert.Equal(t, []string{v.String()}, resp.Header[k])
}
}
46 changes: 23 additions & 23 deletions cmd/query/app/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ import (
"time"

"github.com/spf13/viper"
"go.opentelemetry.io/collector/config/configgrpc"
"go.opentelemetry.io/collector/config/confighttp"
"go.opentelemetry.io/collector/config/configopaque"
"go.uber.org/zap"

"github.com/jaegertracing/jaeger/cmd/query/app/querysvc"
Expand Down Expand Up @@ -57,8 +60,8 @@ type UIConfig struct {
LogAccess bool `mapstructure:"log_access" valid:"optional"`
}

// QueryOptionsBase holds configuration for query service shared with jaeger-v2
type QueryOptionsBase struct {
// QueryOptions holds configuration for query service shared with jaeger-v2
type QueryOptions struct {
// BasePath is the base path for all HTTP routes.
BasePath string `mapstructure:"base_path"`
// UIConfig contains configuration related to the Jaeger UIConfig.
Expand All @@ -71,22 +74,10 @@ type QueryOptionsBase struct {
MaxClockSkewAdjust time.Duration `mapstructure:"max_clock_skew_adjust" valid:"optional"`
// EnableTracing determines whether traces will be emitted by jaeger-query.
EnableTracing bool `mapstructure:"enable_tracing"`
}

// QueryOptions holds configuration for query service
type QueryOptions struct {
QueryOptionsBase

// AdditionalHeaders
AdditionalHeaders http.Header
// HTTPHostPort is the host:port address that the query service listens in on for http requests
HTTPHostPort string
// GRPCHostPort is the host:port address that the query service listens in on for gRPC requests
GRPCHostPort string
// TLSGRPC configures secure transport (Consumer to Query service GRPC API)
TLSGRPC tlscfg.Options
// TLSHTTP configures secure transport (Consumer to Query service HTTP API)
TLSHTTP tlscfg.Options
// HTTP holds the HTTP configuration that the query service uses to serve requests.
HTTP confighttp.ServerConfig `mapstructure:"http"`
// GRPC holds the GRPC configuration that the query service uses to serve requests.
GRPC configgrpc.ServerConfig `mapstructure:"grpc"`
}

// AddFlags adds flags for QueryOptions
Expand All @@ -107,18 +98,18 @@ func AddFlags(flagSet *flag.FlagSet) {

// InitFromViper initializes QueryOptions with properties from viper
func (qOpts *QueryOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) (*QueryOptions, error) {
qOpts.HTTPHostPort = v.GetString(queryHTTPHostPort)
qOpts.GRPCHostPort = v.GetString(queryGRPCHostPort)
qOpts.HTTP.Endpoint = v.GetString(queryHTTPHostPort)
qOpts.GRPC.NetAddr.Endpoint = v.GetString(queryGRPCHostPort)
tlsGrpc, err := tlsGRPCFlagsConfig.InitFromViper(v)
if err != nil {
return qOpts, fmt.Errorf("failed to process gRPC TLS options: %w", err)
}
qOpts.TLSGRPC = tlsGrpc
qOpts.GRPC.TLSSetting = tlsGrpc.ToOtelServerConfig()
tlsHTTP, err := tlsHTTPFlagsConfig.InitFromViper(v)
if err != nil {
return qOpts, fmt.Errorf("failed to process HTTP TLS options: %w", err)
}
qOpts.TLSHTTP = tlsHTTP
qOpts.HTTP.TLSSetting = tlsHTTP.ToOtelServerConfig()
qOpts.BasePath = v.GetString(queryBasePath)
qOpts.UIConfig.AssetsPath = v.GetString(queryStaticFiles)
qOpts.UIConfig.LogAccess = v.GetBool(queryLogStaticAssetsAccess)
Expand All @@ -131,7 +122,7 @@ func (qOpts *QueryOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) (*Q
if err != nil {
logger.Error("Failed to parse headers", zap.Strings("slice", stringSlice), zap.Error(err))
} else {
qOpts.AdditionalHeaders = headers
qOpts.HTTP.ResponseHeaders = mapHTTPHeaderToOTELHeaders(headers)
}
qOpts.Tenancy = tenancy.InitFromViper(v)
qOpts.EnableTracing = v.GetBool(queryEnableTracing)
Expand Down Expand Up @@ -169,3 +160,12 @@ func stringSliceAsHeader(slice []string) (http.Header, error) {

return http.Header(header), nil
}

func mapHTTPHeaderToOTELHeaders(h http.Header) map[string]configopaque.String {
otelHeaders := make(map[string]configopaque.String)
for key, values := range h {
otelHeaders[key] = configopaque.String(strings.Join(values, ","))
}

return otelHeaders
}
20 changes: 10 additions & 10 deletions cmd/query/app/flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@
package app

import (
"net/http"
"strings"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/config/configopaque"
"go.uber.org/zap"

"github.com/jaegertracing/jaeger/pkg/config"
Expand Down Expand Up @@ -39,12 +39,12 @@ func TestQueryBuilderFlags(t *testing.T) {
assert.True(t, qOpts.UIConfig.LogAccess)
assert.Equal(t, "some.json", qOpts.UIConfig.ConfigFile)
assert.Equal(t, "/jaeger", qOpts.BasePath)
assert.Equal(t, "127.0.0.1:8080", qOpts.HTTPHostPort)
assert.Equal(t, "127.0.0.1:8081", qOpts.GRPCHostPort)
assert.Equal(t, http.Header{
"Access-Control-Allow-Origin": []string{"blerg"},
"Whatever": []string{"thing"},
}, qOpts.AdditionalHeaders)
assert.Equal(t, "127.0.0.1:8080", qOpts.HTTP.Endpoint)
assert.Equal(t, "127.0.0.1:8081", qOpts.GRPC.NetAddr.Endpoint)
assert.Equal(t, map[string]configopaque.String{
"Access-Control-Allow-Origin": "blerg",
"Whatever": "thing",
}, qOpts.HTTP.ResponseHeaders)
assert.Equal(t, 10*time.Second, qOpts.MaxClockSkewAdjust)
}

Expand All @@ -55,7 +55,7 @@ func TestQueryBuilderBadHeadersFlags(t *testing.T) {
})
qOpts, err := new(QueryOptions).InitFromViper(v, zap.NewNop())
require.NoError(t, err)
assert.Nil(t, qOpts.AdditionalHeaders)
assert.Nil(t, qOpts.HTTP.ResponseHeaders)
}

func TestStringSliceAsHeader(t *testing.T) {
Expand Down Expand Up @@ -161,8 +161,8 @@ func TestQueryOptionsPortAllocationFromFlags(t *testing.T) {
qOpts, err := new(QueryOptions).InitFromViper(v, zap.NewNop())
require.NoError(t, err)

assert.Equal(t, test.expectedHTTPHostPort, qOpts.HTTPHostPort)
assert.Equal(t, test.expectedGRPCHostPort, qOpts.GRPCHostPort)
assert.Equal(t, test.expectedHTTPHostPort, qOpts.HTTP.Endpoint)
assert.Equal(t, test.expectedGRPCHostPort, qOpts.GRPC.NetAddr.Endpoint)
})
}
}
Expand Down
Loading

0 comments on commit 731f50e

Please sign in to comment.