Feedbacks fixes

Signed-off-by: Ashmita Bohara <ashmita.bohara152@gmail.com>

cmd/anonymizer/app/anonymizer/anonymizer.go

@@ -57,7 +57,7 @@ type Anonymizer struct {
 	logger      *zap.Logger
 	lock        sync.Mutex
 	mapping     mapping
-	options     *Options
+	options     Options
 }
 
 // Options represents the various options with which the anonymizer can be configured.
@@ -70,7 +70,7 @@ type Options struct {
 
 // New creates new Anonymizer. The mappingFile stores the mapping from original to
 // obfuscated strings, in case later investigations require looking at the original traces.
-func New(mappingFile string, options *Options, logger *zap.Logger) *Anonymizer {
+func New(mappingFile string, options Options, logger *zap.Logger) *Anonymizer {
 	a := &Anonymizer{
 		mappingFile: mappingFile,
 		logger:      logger,

cmd/anonymizer/app/anonymizer/anonymizer_test.go

@@ -98,18 +98,17 @@ func TestAnonymizer_Hash(t *testing.T) {
 }
 
 func TestAnonymizer_AnonymizeSpan_AllTrue(t *testing.T) {
-	options := &Options{
-		HashStandardTags: true,
-		HashCustomTags:   true,
-		HashProcess:      true,
-		HashLogs:         true,
-	}
 	anonymizer := &Anonymizer{
 		mapping: mapping{
 			Services:   make(map[string]string),
 			Operations: make(map[string]string),
 		},
-		options: options,
+		options: Options{
+			HashStandardTags: true,
+			HashCustomTags:   true,
+			HashProcess:      true,
+			HashLogs:         true,
+		},
 	}
 	_ = anonymizer.AnonymizeSpan(span1)
 	assert.Equal(t, 3, len(span1.Tags))
@@ -118,18 +117,17 @@ func TestAnonymizer_AnonymizeSpan_AllTrue(t *testing.T) {
 }
 
 func TestAnonymizer_AnonymizeSpan_AllFalse(t *testing.T) {
-	options := &Options{
-		HashStandardTags: false,
-		HashCustomTags:   false,
-		HashProcess:      false,
-		HashLogs:         false,
-	}
 	anonymizer := &Anonymizer{
 		mapping: mapping{
 			Services:   make(map[string]string),
 			Operations: make(map[string]string),
 		},
-		options: options,
+		options: Options{
+			HashStandardTags: false,
+			HashCustomTags:   false,
+			HashProcess:      false,
+			HashLogs:         false,
+		},
 	}
 	_ = anonymizer.AnonymizeSpan(span2)
 	assert.Equal(t, 2, len(span2.Tags))

cmd/anonymizer/app/flags.go

@@ -20,76 +20,69 @@ import (
 
 // Options represent configurable parameters for jaeger-anonymizer
 type Options struct {
-	QueryGRPCPort    int
-	QueryGRPCHost    string
-	MaxSpansCount    int
-	TraceID          string
-	OutputDir        string
-	HashStandardTags bool
-	HashCustomTags   bool
-	HashLogs         bool
-	HashProcess      bool
+	QueryGRPCHostPort string
+	MaxSpansCount     int
+	TraceID           string
+	OutputDir         string
+	HashStandardTags  bool
+	HashCustomTags    bool
+	HashLogs          bool
+	HashProcess       bool
 }
 
 const (
-	queryGRPCHostFlag    = "query-host"
-	queryGRPCPortFlag    = "query-port"
-	outputDirFlag        = "output-dir"
-	traceIDFlag          = "trace-id"
-	hashStandardTagsFlag = "hash-standard-tags"
-	hashCustomTagsFlag   = "hash-custom-tags"
-	hashLogsFlag         = "hash-logs"
-	hashProcessFlag      = "hash-process"
-	maxSpansCount        = "max-spans-count"
+	queryGRPCHostPortFlag = "query-host-port"
+	outputDirFlag         = "output-dir"
+	traceIDFlag           = "trace-id"
+	hashStandardTagsFlag  = "hash-standard-tags"
+	hashCustomTagsFlag    = "hash-custom-tags"
+	hashLogsFlag          = "hash-logs"
+	hashProcessFlag       = "hash-process"
+	maxSpansCount         = "max-spans-count"
 )
 
 // AddFlags adds flags for anonymizer main program
 func (o *Options) AddFlags(command *cobra.Command) {
 	command.Flags().StringVar(
-		&o.QueryGRPCHost,
-		queryGRPCHostFlag,
-		DefaultQueryGRPCHost,
-		"hostname of the jaeger-query endpoint")
-	command.Flags().IntVar(
-		&o.QueryGRPCPort,
-		queryGRPCPortFlag,
-		DefaultQueryGRPCPort,
-		"port of the jaeger-query endpoint")
+		&o.QueryGRPCHostPort,
+		queryGRPCHostPortFlag,
+		"localhost:16686",
+		"The host:port of the jaeger-query endpoint")
 	command.Flags().StringVar(
 		&o.OutputDir,
 		outputDirFlag,
-		DefaultOutputDir,
-		"directory to store the anonymized trace")
+		"/tmp",
+		"The directory to store the anonymized trace")
 	command.Flags().StringVar(
 		&o.TraceID,
 		traceIDFlag,
 		"",
-		"trace-id of trace to anonymize")
+		"The trace-id of trace to anonymize")
 	command.Flags().BoolVar(
 		&o.HashStandardTags,
 		hashStandardTagsFlag,
-		DefaultHashStandardTags,
-		"whether to hash standard tags")
+		false,
+		"Whether to hash standard tags")
 	command.Flags().BoolVar(
 		&o.HashCustomTags,
 		hashCustomTagsFlag,
-		DefaultHashCustomTags,
-		"whether to hash custom tags")
+		false,
+		"Whether to hash custom tags")
 	command.Flags().BoolVar(
 		&o.HashLogs,
 		hashLogsFlag,
-		DefaultHashLogs,
-		"whether to hash logs")
+		false,
+		"Whether to hash logs")
 	command.Flags().BoolVar(
 		&o.HashProcess,
 		hashProcessFlag,
-		DefaultHashProcess,
-		"whether to hash process")
+		false,
+		"Whether to hash process")
 	command.Flags().IntVar(
 		&o.MaxSpansCount,
 		maxSpansCount,
-		DefaultMaxSpansCount,
-		"maximum number of spans to anonymize")
+		-1,
+		"The maximum number of spans to anonymize")
 
 	// mark traceid flag as mandatory
 	command.MarkFlagRequired(traceIDFlag)

cmd/anonymizer/app/query/.nocover

@@ -0,0 +1 @@
+non-critical test utility

cmd/anonymizer/app/query/query.go

@@ -16,6 +16,7 @@ package query
 
 import (
 	"context"
+	"fmt"
 	"time"
 
 	"go.uber.org/zap"
@@ -26,66 +27,52 @@ import (
 	"github.com/jaegertracing/jaeger/storage/spanstore"
 )
 
-// GrpcClient represents a grpc-client to jaeger-query
-type GrpcClient struct {
-	api_v2.QueryServiceClient
-	conn *grpc.ClientConn
-}
-
 // Query represents a jaeger-query's query for trace-id
 type Query struct {
-	grpcClient *GrpcClient
-	logger     *zap.Logger
+	api_v2.QueryServiceClient
+	conn   *grpc.ClientConn
+	logger *zap.Logger
 }
 
-// newGRPCClient returns a new GrpcClient
-func newGRPCClient(addr string, logger *zap.Logger) *GrpcClient {
+// New creates a Query object
+func New(addr string, logger *zap.Logger) (*Query, error) {
 	ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second)
 	defer cancel()
 
 	conn, err := grpc.DialContext(ctx, addr, grpc.WithInsecure())
 	if err != nil {
-		logger.Fatal("failed to connect with the jaeger-query service", zap.Error(err))
+		return nil, fmt.Errorf("failed to connect with the jaeger-query service: %w", err)
 	}
 
-	return &GrpcClient{
+	return &Query{
 		QueryServiceClient: api_v2.NewQueryServiceClient(conn),
 		conn:               conn,
-	}
-}
-
-// New creates a Query object
-func New(addr string, logger *zap.Logger) (*Query, error) {
-	client := newGRPCClient(addr, logger)
-
-	return &Query{
-		grpcClient: client,
-		logger:     logger,
+		logger:             logger,
 	}, nil
 }
 
 // QueryTrace queries for a trace and returns all spans inside it
-func (q *Query) QueryTrace(traceID string) []model.Span {
+func (q *Query) QueryTrace(traceID string) ([]model.Span, error) {
 	mTraceID, err := model.TraceIDFromString(traceID)
 	if err != nil {
-		q.logger.Fatal("failed to convert the provided trace id", zap.Error(err))
+		return nil, fmt.Errorf("failed to convert the provided trace id: %w", err)
 	}
 
-	response, err := q.grpcClient.GetTrace(context.Background(), &api_v2.GetTraceRequest{
+	response, err := q.GetTrace(context.Background(), &api_v2.GetTraceRequest{
 		TraceID: mTraceID,
 	})
 	if err != nil {
-		q.logger.Fatal("failed to fetch the provided trace id", zap.Error(err))
+		return nil, fmt.Errorf("failed to fetch the provided trace id: %w", err)
 	}
 
 	spanResponseChunk, err := response.Recv()
 	if err == spanstore.ErrTraceNotFound {
-		q.logger.Fatal("failed to find the provided trace id", zap.Error(err))
+		return nil, fmt.Errorf("failed to find the provided trace id: %w", err)
 	}
 	if err != nil {
-		q.logger.Fatal("failed to fetch spans of provided trace id", zap.Error(err))
+		return nil, fmt.Errorf("failed to fetch spans of provided trace id: %w", err)
 	}
 
 	spans := spanResponseChunk.GetSpans()
-	return spans
+	return spans, nil
 }

cmd/anonymizer/app/writer/writer.go

@@ -30,14 +30,11 @@ import (
 
 // Config contains parameters to NewWriter.
 type Config struct {
-	MaxSpansCount    int    `yaml:"max_spans_count" name:"max_spans_count"`
-	CapturedFile     string `yaml:"captured_file" name:"captured_file"`
-	AnonymizedFile   string `yaml:"anonymized_file" name:"anonymized_file"`
-	MappingFile      string `yaml:"mapping_file" name:"mapping_file"`
-	HashStandardTags bool   `yaml:"hash_standard_tags" name:"hash_standard_tags"`
-	HashCustomTags   bool   `yaml:"hash_custom_tags" name:"hash_custom_tags"`
-	HashLogs         bool   `yaml:"hash_logs" name:"hash_logs"`
-	HashProcess      bool   `yaml:"hash_process" name:"hash_process"`
+	MaxSpansCount  int    `yaml:"max_spans_count" name:"max_spans_count"`
+	CapturedFile   string `yaml:"captured_file" name:"captured_file"`
+	AnonymizedFile string `yaml:"anonymized_file" name:"anonymized_file"`
+	MappingFile    string `yaml:"mapping_file" name:"mapping_file"`
+	AnonymizerOpts anonymizer.Options
 }
 
 // Writer is a span Writer that obfuscates the span and writes it to a JSON file.
@@ -80,11 +77,11 @@ func New(config Config, logger *zap.Logger) (*Writer, error) {
 		return nil, fmt.Errorf("cannot write tp output file: %w", err)
 	}
 
-	options := &anonymizer.Options{
-		HashStandardTags: config.HashStandardTags,
-		HashCustomTags:   config.HashCustomTags,
-		HashLogs:         config.HashLogs,
-		HashProcess:      config.HashProcess,
+	options := anonymizer.Options{
+		HashStandardTags: config.AnonymizerOpts.HashStandardTags,
+		HashCustomTags:   config.AnonymizerOpts.HashCustomTags,
+		HashLogs:         config.AnonymizerOpts.HashLogs,
+		HashProcess:      config.AnonymizerOpts.HashProcess,
 	}
 
 	return &Writer{