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

[query] Change HTTP and TLS server configurations to use OTEL configurations #6023

Merged
merged 11 commits into from
Sep 28, 2024
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) {
yurishkuro marked this conversation as resolved.
Show resolved Hide resolved
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")
yurishkuro marked this conversation as resolved.
Show resolved Hide resolved
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
yurishkuro marked this conversation as resolved.
Show resolved Hide resolved
// 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
Loading