Skip to content

Commit

Permalink
all: remove Trace and SpanContext from SearchOptions
Browse files Browse the repository at this point in the history
Since the move to gRPC these fields should make no difference.
SpanContext is never set anymore by our clients. Trace is still set, but
I believe we should be communicating to trace or not via grpc.

Additionally we just rely on the opentracing global tracer. If tracing
is not configured it will just use the noop tracer, not sure why we had
our own implementation.

Test Plan: go test. Will seek more guidance on other tests to run.
  • Loading branch information
keegancsmith committed Apr 19, 2024
1 parent a0f051e commit dd033e3
Show file tree
Hide file tree
Showing 10 changed files with 386 additions and 527 deletions.
12 changes: 0 additions & 12 deletions api.go
Original file line number Diff line number Diff line change
Expand Up @@ -951,15 +951,8 @@ type SearchOptions struct {
// When enabled, all other scoring signals are ignored, including document ranks.
UseKeywordScoring bool

// Trace turns on opentracing for this request if true and if the Jaeger address was provided as
// a command-line flag
Trace bool

// If set, the search results will contain debug information for scoring.
DebugScore bool

// SpanContext is the opentracing span context, if it exists, from the zoekt client
SpanContext map[string]string
}

// String returns a succinct representation of the options. This is meant for
Expand Down Expand Up @@ -1016,13 +1009,8 @@ func (s *SearchOptions) String() string {
addBool("ChunkMatches", s.ChunkMatches)
addBool("UseDocumentRanks", s.UseDocumentRanks)
addBool("UseKeywordScoring", s.UseKeywordScoring)
addBool("Trace", s.Trace)
addBool("DebugScore", s.DebugScore)

for k, v := range s.SpanContext {
add("SpanContext."+k, strconv.Quote(v))
}

b.WriteByte('}')
return b.String()
}
Expand Down
2 changes: 0 additions & 2 deletions api_proto.go
Original file line number Diff line number Diff line change
Expand Up @@ -698,7 +698,6 @@ func SearchOptionsFromProto(p *proto.SearchOptions) *SearchOptions {
ChunkMatches: p.GetChunkMatches(),
UseDocumentRanks: p.GetUseDocumentRanks(),
DocumentRanksWeight: p.GetDocumentRanksWeight(),
Trace: p.GetTrace(),
DebugScore: p.GetDebugScore(),
UseKeywordScoring: p.GetUseKeywordScoring(),
}
Expand All @@ -723,7 +722,6 @@ func (s *SearchOptions) ToProto() *proto.SearchOptions {
ChunkMatches: s.ChunkMatches,
UseDocumentRanks: s.UseDocumentRanks,
DocumentRanksWeight: s.DocumentRanksWeight,
Trace: s.Trace,
DebugScore: s.DebugScore,
UseKeywordScoring: s.UseKeywordScoring,
}
Expand Down
3 changes: 0 additions & 3 deletions api_proto_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -334,9 +334,6 @@ func TestProtoRoundtrip(t *testing.T) {

t.Run("SearchOptions", func(t *testing.T) {
f := func(f1 *SearchOptions) bool {
if f1 != nil {
f1.SpanContext = nil
}
p1 := f1.ToProto()
f2 := SearchOptionsFromProto(p1)
if diff := cmp.Diff(f1, f2); diff != "" {
Expand Down
9 changes: 2 additions & 7 deletions cmd/zoekt-webserver/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,10 +293,7 @@ func main() {
}

logger := sglog.Scoped("ZoektWebserverGRPCServer")

streamer := web.NewTraceAwareSearcher(s.Searcher)
grpcServer := newGRPCServer(logger, streamer)

grpcServer := newGRPCServer(logger, s.Searcher)
handler = multiplexGRPC(grpcServer, handler)

srv := &http.Server{
Expand Down Expand Up @@ -638,7 +635,7 @@ func traceContext(ctx context.Context) sglog.TraceContext {
return sglog.TraceContext{}
}

func newGRPCServer(logger sglog.Logger, streamer zoekt.Streamer, additionalOpts ...grpc.ServerOption) *grpc.Server {
func newGRPCServer(logger sglog.Logger, streamer zoekt.Streamer) *grpc.Server {
metrics := mustGetServerMetrics()

opts := []grpc.ServerOption{
Expand All @@ -656,8 +653,6 @@ func newGRPCServer(logger sglog.Logger, streamer zoekt.Streamer, additionalOpts
),
}

opts = append(opts, additionalOpts...)

// Ensure that the message size options are set last, so they override any other
// server-specific options that tweak the message size.
//
Expand Down
767 changes: 378 additions & 389 deletions grpc/protos/zoekt/webserver/v1/webserver.pb.go

Large diffs are not rendered by default.

7 changes: 3 additions & 4 deletions grpc/protos/zoekt/webserver/v1/webserver.proto
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ message StreamSearchResponse {
}

message SearchOptions {
reserved 13;
reserved "trace";

// Return an upper-bound estimate of eligible documents in
// stats.ShardFilesConsidered.
bool estimate_doc_count = 1;
Expand Down Expand Up @@ -100,10 +103,6 @@ message SearchOptions {
// will be used. This option is temporary and is only exposed for testing/ tuning purposes.
double document_ranks_weight = 12;

// Trace turns on opentracing for this request if true and if the Jaeger address was provided as
// a command-line flag
bool trace = 13;

// If set, the search results will contain debug information for scoring.
bool debug_score = 14;

Expand Down
49 changes: 0 additions & 49 deletions trace/opentracing.go

This file was deleted.

4 changes: 2 additions & 2 deletions trace/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,13 @@ type Tracer struct {
}

func New(ctx context.Context, family, title string) (*Trace, context.Context) {
tr := Tracer{Tracer: GetOpenTracer(ctx, nil)}
tr := Tracer{Tracer: opentracing.GlobalTracer()}
return tr.New(ctx, family, title)
}

// New returns a new Trace with the specified family and title.
func (t Tracer) New(ctx context.Context, family, title string) (*Trace, context.Context) {
span, ctx := StartSpanFromContextWithTracer(
span, ctx := opentracing.StartSpanFromContextWithTracer(
ctx,
t.Tracer,
family,
Expand Down
2 changes: 1 addition & 1 deletion web/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ func NewMux(s *Server) (*http.ServeMux, error) {
mux.HandleFunc("/print", s.servePrint)
}
if s.RPC {
mux.Handle("/api/", http.StripPrefix("/api", zjson.JSONServer(traceAwareSearcher{s.Searcher})))
mux.Handle("/api/", http.StripPrefix("/api", zjson.JSONServer(s.Searcher)))
}

mux.HandleFunc("/healthz", s.serveHealthz)
Expand Down
58 changes: 0 additions & 58 deletions web/trace.go

This file was deleted.

0 comments on commit dd033e3

Please sign in to comment.