Addressed comments, added tests

Signed-off-by: Annanay <annanay.a@media.net>
jaegertracing · Mar 13, 2019 · 2b8d41e · 2b8d41e
1 parent bea5bff
commit 2b8d41e
Show file tree

Hide file tree

Showing 5 changed files with 51 additions and 17 deletions.
diff --git a/cmd/agent/app/reporter/flags.go b/cmd/agent/app/reporter/flags.go
@@ -27,7 +27,7 @@ const (
 	// Whether to use grpc or tchannel reporter.
 	reporterType = "reporter.type"
 	// Agent tags
-	agentTags = "jaeger.tag"
+	agentTags = "jaeger.tags"
 	// TCHANNEL is name of tchannel reporter.
 	TCHANNEL Type = "tchannel"
 	// GRPC is name of gRPC reporter.
@@ -46,7 +46,7 @@ type Options struct {
 // AddFlags adds flags for Options.
 func AddFlags(flags *flag.FlagSet) {
 	flags.String(reporterType, string(GRPC), fmt.Sprintf("Reporter type to use e.g. %s, %s", string(GRPC), string(TCHANNEL)))
-	flags.String(agentTags, "", fmt.Sprintf("Add agent-based tags. Ex: --jaeger.tag=key1=value1,${envVar:defaultValue}"))
+	flags.String(agentTags, "", fmt.Sprintf("Add agent-based tags. Ex: --jaeger.tags=key1=value1,${envVar:defaultValue}"))
 }
 
 // InitFromViper initializes Options with properties retrieved from Viper.

diff --git a/cmd/agent/app/reporter/flags_test.go b/cmd/agent/app/reporter/flags_test.go
@@ -16,6 +16,7 @@ package reporter
 
 import (
 	"flag"
+	"os"
 	"testing"
 
 	"github.com/spf13/cobra"
@@ -24,6 +25,25 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+func TestBindFlags_NoJaegerTags(t *testing.T) {
+	v := viper.New()
+	command := cobra.Command{}
+	flags := &flag.FlagSet{}
+	AddFlags(flags)
+	command.PersistentFlags().AddGoFlagSet(flags)
+	v.BindPFlags(command.PersistentFlags())
+
+	err := command.ParseFlags([]string{
+		"--reporter.type=grpc",
+	})
+	require.NoError(t, err)
+
+	b := &Options{}
+	b.InitFromViper(v)
+	assert.Equal(t, Type("grpc"), b.ReporterType)
+	assert.Equal(t, parseAgentTags(""), b.AgentTags)
+}
+
 func TestBindFlags(t *testing.T) {
 	v := viper.New()
 	command := cobra.Command{}
@@ -34,12 +54,15 @@ func TestBindFlags(t *testing.T) {
 
 	err := command.ParseFlags([]string{
 		"--reporter.type=grpc",
-		"--jaeger.tag=key=value",
+		"--jaeger.tags=key=value,envVar1=${envKey1:defaultVal1},envVar2=${envKey2:defaultVal2}",
 	})
 	require.NoError(t, err)
 
 	b := &Options{}
+	os.Setenv("envKey1","envVal1")
 	b.InitFromViper(v)
+
 	assert.Equal(t, Type("grpc"), b.ReporterType)
-	assert.Equal(t, parseAgentTags("key=value"), b.AgentTags)
+	assert.Equal(t, parseAgentTags("key=value,envVar1=${envKey1:defaultVal1},envVar2=${envKey2:defaultVal2}"), b.AgentTags)
+	os.Unsetenv("envKey1")
 }
diff --git a/cmd/agent/app/reporter/grpc/reporter.go b/cmd/agent/app/reporter/grpc/reporter.go
@@ -16,6 +16,7 @@ package grpc
 
 import (
 	"context"
+	"fmt"
 
 	"go.uber.org/zap"
 	"google.golang.org/grpc"
@@ -65,7 +66,7 @@ func (r *Reporter) EmitZipkinBatch(zSpans []*zipkincore.Span) error {
 }
 
 func (r *Reporter) send(spans []*model.Span, process *model.Process) error {
-	spans = addTags(spans, r.agentTags)
+	spans = addProcessTags(spans, r.agentTags)
 	batch := model.Batch{Spans: spans, Process: process}
 	req := &api_v2.PostSpansRequest{Batch: batch}
 	_, err := r.collector.PostSpans(context.Background(), req)
@@ -76,20 +77,24 @@ func (r *Reporter) send(spans []*model.Span, process *model.Process) error {
 }
 
 // addTags appends jaeger tags for the agent to every span it sends to the collector.
-func addTags(spans []*model.Span, agentTags []model.KeyValue) []*model.Span {
+func addProcessTags(spans []*model.Span, agentTags []model.KeyValue) []*model.Span {
 	for _, span := range spans {
-		span.Tags = append(span.Tags, agentTags...)
+		if len(agentTags) > 0 {
+			if span.Process == nil {
+				span.Process = &model.Process{Tags: agentTags}
+			} else {
+				span.Process.Tags = append(span.Process.Tags, agentTags...)
+			}
+		}
 	}
 	return spans
 }
 
 func makeModelKeyValue(agentTags map[string]string) []model.KeyValue {
-	tags := make([]model.KeyValue, len(agentTags))
+	tags := []model.KeyValue{}
 	for k, v := range agentTags {
-		tag := model.KeyValue{
-			Key:  k,
-			VStr: v,
-		}
+		fmt.Println("Adding", k, v)
+		tag := model.String(k, v)
 		tags = append(tags, tag)
 	}
 

diff --git a/cmd/agent/app/reporter/grpc/reporter_test.go b/cmd/agent/app/reporter/grpc/reporter_test.go
@@ -124,3 +124,13 @@ func TestReporter_SendFailure(t *testing.T) {
 	err = rep.send(nil, nil)
 	assert.EqualError(t, err, "rpc error: code = Unavailable desc = all SubConns are in TransientFailure, latest connection error: connection error: desc = \"transport: Error while dialing dial tcp: missing address\"")
 }
+
+func TestReporter_MakeModelKeyValue(t *testing.T) {
+	stringTags := make(map[string]string)
+	stringTags["hello"] = "world"
+	jaegerTags := makeModelKeyValue(stringTags)
+	expected := []model.KeyValue{model.String("hello","world")}
+
+	assert.Equal(t, 1, len(jaegerTags))
+	assert.Equal(t, expected, jaegerTags)
+}
diff --git a/cmd/agent/app/reporter/tchannel/reporter.go b/cmd/agent/app/reporter/tchannel/reporter.go
@@ -26,10 +26,6 @@ import (
 	"github.com/jaegertracing/jaeger/thrift-gen/zipkincore"
 )
 
-const (
-	annotationTypeStr = 6
-)
-
 // Reporter forwards received spans to central collector tier over TChannel.
 type Reporter struct {
 	channel       *tchannel.Channel
@@ -123,7 +119,7 @@ func addAgentTagsToZipkinBatch(spans []*zipkincore.Span, agentTags []jaeger.Tag)
 			span.BinaryAnnotations = append(span.BinaryAnnotations, &zipkincore.BinaryAnnotation{
 				Key:            tag.Key,
 				Value:          []byte(*tag.VStr),
-				AnnotationType: annotationTypeStr, // static value set to string type.
+				AnnotationType: zipkincore.AnnotationType_STRING, // static value set to string type.
 			})
 		}
 	}