Skip to content

Commit 056bc1d

Browse files
authored
Add more unit tests for creating metrics, fix bug related to unmarshalling flags (#5169)
* fix bug with unmarshalling flags and add test * test that otel metrics are created
1 parent 531adb6 commit 056bc1d

File tree

4 files changed

+280
-72
lines changed

4 files changed

+280
-72
lines changed

go.mod

+2-1
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,10 @@ require (
6868
github.com/spf13/pflag v1.0.5
6969
github.com/tektoncd/pipeline v0.5.1-0.20190731183258-9d7e37e85bf8
7070
github.com/xeipuuv/gojsonschema v1.2.0
71-
golang.org/x/crypto v0.0.0-20201117144127-c1f2f97bffc9
7271
go.opentelemetry.io/otel v0.13.0
72+
go.opentelemetry.io/otel/exporters/stdout v0.13.0
7373
go.opentelemetry.io/otel/sdk v0.13.0
74+
golang.org/x/crypto v0.0.0-20201117144127-c1f2f97bffc9
7475
golang.org/x/mod v0.3.0
7576
golang.org/x/net v0.0.0-20201202161906-c7110b5ffcbb // indirect
7677
golang.org/x/oauth2 v0.0.0-20201109201403-9fd604954f58

go.sum

+9
Original file line numberDiff line numberDiff line change
@@ -1274,10 +1274,19 @@ go.opentelemetry.io/otel v0.13.0 h1:2isEnyzjjJZq6r2EKMsFj4TxiQiexsM04AVhwbR/oBA=
12741274
go.opentelemetry.io/otel v0.13.0/go.mod h1:dlSNewoRYikTkotEnxdmuBHgzT+k/idJSfDv/FxEnOY=
12751275
go.opentelemetry.io/otel v0.14.0 h1:YFBEfjCk9MTjaytCNSUkp9Q8lF7QJezA06T71FbQxLQ=
12761276
go.opentelemetry.io/otel v0.14.0/go.mod h1:vH5xEuwy7Rts0GNtsCW3HYQoZDY+OmBJ6t1bFGGlxgw=
1277+
go.opentelemetry.io/otel v0.15.0 h1:CZFy2lPhxd4HlhZnYK8gRyDotksO3Ip9rBweY1vVYJw=
1278+
go.opentelemetry.io/otel v0.15.0/go.mod h1:e4GKElweB8W2gWUqbghw0B8t5MCTccc9212eNHnOHwA=
1279+
go.opentelemetry.io/otel/exporters/stdout v0.13.0 h1:A+XiGIPQbGoJoBOJfKAKnZyiUSjSWvL3XWETUvtom5k=
1280+
go.opentelemetry.io/otel/exporters/stdout v0.13.0/go.mod h1:JJt8RpNY6K+ft9ir3iKpceCvT/rhzJXEExGrWFCbv1o=
1281+
go.opentelemetry.io/otel/exporters/stdout v0.14.0/go.mod h1:KG9w470+KbZZexYbC/g3TPKgluS0VgBJHh4KlnJpG18=
1282+
go.opentelemetry.io/otel/exporters/stdout v0.15.0 h1:/i7NvRnB+L7R/uxwpfolovicyBFnFa527NBs2yIhPUo=
1283+
go.opentelemetry.io/otel/exporters/stdout v0.15.0/go.mod h1:1d+FA51tyW9NDD0VXUsk5K5S3LAOt9GBWU3TNelHhxA=
12771284
go.opentelemetry.io/otel/sdk v0.13.0 h1:4VCfpKamZ8GtnepXxMRurSpHpMKkcxhtO33z1S4rGDQ=
12781285
go.opentelemetry.io/otel/sdk v0.13.0/go.mod h1:dKvLH8Uu8LcEPlSAUsfW7kMGaJBhk/1NYvpPZ6wIMbU=
12791286
go.opentelemetry.io/otel/sdk v0.14.0 h1:Pqgd85y5XhyvHQlOxkKW+FD4DAX7AoeaNIDKC2VhfHQ=
12801287
go.opentelemetry.io/otel/sdk v0.14.0/go.mod h1:kGO5pEMSNqSJppHAm8b73zztLxB5fgDQnD56/dl5xqE=
1288+
go.opentelemetry.io/otel/sdk v0.15.0 h1:Hf2dl1Ad9Hn03qjcAuAq51GP5Pv1SV5puIkS2nRhdd8=
1289+
go.opentelemetry.io/otel/sdk v0.15.0/go.mod h1:Qudkwgq81OcA9GYVlbyZ62wkLieeS1eWxIL0ufxgwoc=
12811290
go.starlark.net v0.0.0-20190528202925-30ae18b8564f/go.mod h1:c1/X6cHgvdXj6pUlmWKMkuqRnW4K8x2vwt6JAaaircg=
12821291
go.starlark.net v0.0.0-20200306205701-8dd3e2ee1dd5/go.mod h1:nmDLcffg48OtT/PSW0Hg7FvpRQsQh5OSqIylirxKC7o=
12831292
go.uber.org/atomic v1.3.2/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE=

pkg/skaffold/instrumentation/meter.go

+22-24
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ import (
4242

4343
"github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/cmd/statik"
4444

45-
// import embedded secret for uploading metrics
45+
// import embedded secret for uploading metrics
4646
_ "github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/secret/statik"
4747
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
4848
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
@@ -61,7 +61,7 @@ type skaffoldMeter struct {
6161
Arch string
6262
PlatformType string
6363
Deployers []string
64-
EnumFlags map[string]*flag.Flag
64+
EnumFlags map[string]string
6565
Builders map[string]int
6666
SyncType map[string]bool
6767
DevIterations []devIteration
@@ -71,15 +71,15 @@ type skaffoldMeter struct {
7171
}
7272

7373
type devIteration struct {
74-
intent string
75-
errorCode proto.StatusCode
74+
Intent string
75+
ErrorCode proto.StatusCode
7676
}
7777

7878
var (
7979
meter = skaffoldMeter{
8080
OS: runtime.GOOS,
8181
Arch: runtime.GOARCH,
82-
EnumFlags: map[string]*flag.Flag{},
82+
EnumFlags: map[string]string{},
8383
Builders: map[string]int{},
8484
SyncType: map[string]bool{},
8585
DevIterations: []devIteration{},
@@ -92,6 +92,7 @@ var (
9292
meteredCommands = util.NewStringSet()
9393
doesBuild = util.NewStringSet()
9494
doesDeploy = util.NewStringSet()
95+
initExporter = initCloudMonitoringExporterMetrics
9596
isOnline bool
9697
)
9798

@@ -113,11 +114,7 @@ func init() {
113114
func InitMeterFromConfig(config *latest.SkaffoldConfig) {
114115
meter.PlatformType = yamltags.GetYamlTag(config.Build.BuildType)
115116
for _, artifact := range config.Pipeline.Build.Artifacts {
116-
if _, ok := meter.Builders[yamltags.GetYamlTag(artifact.ArtifactType)]; ok {
117-
meter.Builders[yamltags.GetYamlTag(artifact.ArtifactType)]++
118-
} else {
119-
meter.Builders[yamltags.GetYamlTag(artifact.ArtifactType)] = 1
120-
}
117+
meter.Builders[yamltags.GetYamlTag(artifact.ArtifactType)]++
121118
if artifact.Sync != nil {
122119
meter.SyncType[yamltags.GetYamlTag(artifact.Sync)] = true
123120
}
@@ -137,18 +134,20 @@ func SetErrorCode(errorCode proto.StatusCode) {
137134
}
138135

139136
func AddDevIteration(intent string) {
140-
meter.DevIterations = append(meter.DevIterations, devIteration{intent: intent})
137+
meter.DevIterations = append(meter.DevIterations, devIteration{Intent: intent})
141138
}
142139

143140
func AddDevIterationErr(i int, errorCode proto.StatusCode) {
144141
if len(meter.DevIterations) == i {
145-
meter.DevIterations = append(meter.DevIterations, devIteration{intent: "error"})
142+
meter.DevIterations = append(meter.DevIterations, devIteration{Intent: "error"})
146143
}
147-
meter.DevIterations[i].errorCode = errorCode
144+
meter.DevIterations[i].ErrorCode = errorCode
148145
}
149146

150147
func AddFlag(flag *flag.Flag) {
151-
meter.EnumFlags[flag.Name] = flag
148+
if flag.Changed {
149+
meter.EnumFlags[flag.Name] = flag.Value.String()
150+
}
152151
}
153152

154153
func ExportMetrics(exitCode int) error {
@@ -168,7 +167,7 @@ func ExportMetrics(exitCode int) error {
168167

169168
func exportMetrics(ctx context.Context, filename string, meter skaffoldMeter) error {
170169
logrus.Debug("exporting metrics")
171-
p, err := initCloudMonitoringExporterMetrics()
170+
p, err := initExporter()
172171
if p == nil {
173172
return err
174173
}
@@ -189,11 +188,13 @@ func exportMetrics(ctx context.Context, filename string, meter skaffoldMeter) er
189188
return ioutil.WriteFile(filename, b, 0666)
190189
}
191190

191+
start := time.Now()
192192
p.Start()
193193
for _, m := range meters {
194194
createMetrics(ctx, m)
195195
}
196196
p.Stop()
197+
logrus.Debugf("metrics uploading complete in %s", time.Since(start).String())
197198

198199
if fileExists {
199200
return os.Remove(filename)
@@ -221,8 +222,8 @@ func initCloudMonitoringExporterMetrics() (*push.Controller, error) {
221222

222223
var c creds
223224
err = json.Unmarshal(b, &c)
224-
if err != nil {
225-
return nil, fmt.Errorf("error unmarsharling metrics credentials: %v", err)
225+
if c.ProjectID == "" || err != nil {
226+
return nil, fmt.Errorf("no project id found in metrics credentials")
226227
}
227228

228229
formatter := func(desc *metric.Descriptor) string {
@@ -295,14 +296,11 @@ func commandMetrics(ctx context.Context, meter skaffoldMeter, m metric.Meter, ra
295296
counts := make(map[string]map[proto.StatusCode]int)
296297

297298
for _, iteration := range meter.DevIterations {
298-
if _, ok := counts[iteration.intent]; !ok {
299-
counts[iteration.intent] = make(map[proto.StatusCode]int)
300-
}
301-
m := counts[iteration.intent]
302-
if _, ok := m[iteration.errorCode]; !ok {
303-
m[iteration.errorCode] = 0
299+
if _, ok := counts[iteration.Intent]; !ok {
300+
counts[iteration.Intent] = make(map[proto.StatusCode]int)
304301
}
305-
m[iteration.errorCode]++
302+
m := counts[iteration.Intent]
303+
m[iteration.ErrorCode]++
306304
}
307305
for intention, errorCounts := range counts {
308306
for errorCode, count := range errorCounts {

0 commit comments

Comments
 (0)