improve stackdriver metric type #1132

cyriltovena · 2019-10-22T18:37:41Z

As per the documentation https://godoc.org/contrib.go.opencensus.io/exporter/stackdriver

This will improve the metric typing and should now appears as a gke container in stackdriver interface.

If someone has time to test it and let me know if everything is correct, that would be awesome.

agones-bot · 2019-10-22T19:13:53Z

Build Succeeded 👏

Build Id: 22468608-4412-4e90-b581-e2ce5ea32126

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:1.1.0-662ea70
image: gcr.io/agones-images/agones-sdk:1.1.0-662ea70
image: gcr.io/agones-images/agones-ping:1.1.0-662ea70
Linux C++ SDK (build): agonessdk-1.1.0-662ea70-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.1.0-662ea70.zip

A preview of the website (the last 30 builds are retained):

https://662ea70-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1132/head:pr_1132 && git checkout pr_1132
helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.1.0-662ea70

cyriltovena · 2019-10-23T00:52:10Z

Why the tag approved?

markmandel · 2019-10-23T18:12:09Z

I think prow approves since you are an approver? @roberthbailey ?

roberthbailey · 2019-10-23T19:34:15Z

Why the tag approved?

Yes, that is from prow. If you weren't an approver, the tag could be added by an approver typing /approve in a PR comment or by approving via the github code review flow.

Right now it doesn't mean anything, but if we decide to switch to tide for automatic merges then it'll be required for a PR to be merged.

pkg/metrics/exporter.go

roberthbailey · 2019-10-28T17:57:58Z

/assign

agones-bot · 2019-10-29T13:12:55Z

Build Succeeded 👏

Build Id: 7915ed5c-f278-466a-adaa-3e4763d20f9d

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:1.1.0-d1528f0
image: gcr.io/agones-images/agones-sdk:1.1.0-d1528f0
image: gcr.io/agones-images/agones-ping:1.1.0-d1528f0
Linux C++ SDK (build): agonessdk-1.1.0-d1528f0-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.1.0-d1528f0.zip

A preview of the website (the last 30 builds are retained):

https://d1528f0-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1132/head:pr_1132 && git checkout pr_1132
helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.1.0-d1528f0

agones-bot · 2019-11-06T00:24:14Z

Build Failed 😱

Build Id: e2651f72-6826-403a-9a57-e2b2544af104

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

agones-bot · 2019-11-06T00:39:18Z

Build Failed 😱

Build Id: bffee03d-008e-4d6f-a4b9-9db3aca058ad

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

agones-bot · 2019-11-06T12:16:51Z

Build Succeeded 👏

Build Id: aaa27268-7850-4af2-aed4-baec00cd1b74

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:1.1.0-30b4daf
image: gcr.io/agones-images/agones-sdk:1.1.0-30b4daf
image: gcr.io/agones-images/agones-ping:1.1.0-30b4daf
Linux C++ SDK (build): agonessdk-1.1.0-30b4daf-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.1.0-30b4daf.zip

A preview of the website (the last 30 builds are retained):

https://30b4daf-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1132/head:pr_1132 && git checkout pr_1132
helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.1.0-30b4daf

pkg/metrics/exporter.go

agones-bot · 2019-11-06T13:09:41Z

Build Succeeded 👏

Build Id: 077c203c-270f-4761-841a-9477cc298375

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:1.1.0-85873c1
image: gcr.io/agones-images/agones-sdk:1.1.0-85873c1
image: gcr.io/agones-images/agones-ping:1.1.0-85873c1
Linux C++ SDK (build): agonessdk-1.1.0-85873c1-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.1.0-85873c1.zip

A preview of the website (the last 30 builds are retained):

https://85873c1-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1132/head:pr_1132 && git checkout pr_1132
helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.1.0-85873c1

roberthbailey · 2019-11-07T06:51:24Z

@cyriltovena - is this ready for another review pass?

cyriltovena · 2019-11-07T09:57:49Z

Yes !

site/content/en/docs/Installation/helm.md

roberthbailey · 2019-11-08T06:51:09Z

pkg/metrics/util.go

+		if len(keyValue) != 2 {
+			return nil, fmt.Errorf("invalid labels format: %s, expect key=value,key2=value2", s)
+		}
+		res.Set(keyValue[0], keyValue[1], "")


should this trim whitespace around the labels so that something like key=value, key2=value2 doesn't produce a label named key2 (with a space at the beginning)?

roberthbailey · 2019-11-08T06:51:36Z

pkg/metrics/util.go

+		return res, nil
+	}
+	pairs := strings.Split(s, ",")
+	if len(pairs) == 0 {


There isn't a test case for this branch.

roberthbailey · 2019-11-08T07:01:14Z

pkg/metrics/util_test.go

@@ -291,3 +293,57 @@ agones_gameservers_node_count_count 3
 agones_nodes_count{empty="false"} 2
 agones_nodes_count{empty="true"} 1
 `
+
+func Test_parseLabels(t *testing.T) {
+	tests := []struct {


Another way you could write this (to more easily explore edge cases that should create errors) is to have two sets of tests, one that should produce errors and another that should work. I think the set of tests that should work is relatively small (you have 3 right now, which mostly covers it). But the set of inputs that should cause an error should be made much longer. This is easier if it's more compact, e.g.

errorInputs = []string{ "a=b,", "a=b,c", ... }

since the want is always nil and the wantErr is always true for these you can collapse them into a shorter list form, and then expand the list.

What about testing inputs with spaces added in various places? Or extra equals signs? Should non-ascii characters be accepted? Does stack driver impose any constraints on what characters can be in a key? And so on.

roberthbailey · 2019-11-08T07:04:32Z

pkg/metrics/exporter.go

+			Type: "k8s_container",
+			Labels: map[string]string{
+				"project_id":   projectID,
+				"instance_id":  instanceID,


Do these have to be set? Or can they be left blank if we can't read from the metadata server?

agones-bot · 2019-12-08T20:56:00Z

Build Failed 😱

Build Id: ef193702-513e-4c35-81c2-beec9c5c23a5

To get permission to view the Cloud Build view, join the agones-discuss Google Group.

markmandel · 2019-12-08T20:58:06Z

========================================================================
site/docs/installation/upgrading/index.html
  Non-OK status: 404 --- site/docs/installation/upgrading/index.html --> https://helm.sh/docs/helm/#helm-upgrade
========================================================================

Rebase / Upgrade to master - should solve that problem.

agones-bot · 2019-12-08T21:03:41Z

Build Succeeded 👏

Build Id: 54295db5-d68a-45f9-b5a7-88351902455f

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:1.2.0-d26abb2
image: gcr.io/agones-images/agones-sdk:1.2.0-d26abb2
image: gcr.io/agones-images/agones-ping:1.2.0-d26abb2
Linux C++ SDK (build): agonessdk-1.2.0-d26abb2-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.2.0-d26abb2.zip

A preview of the website (the last 30 builds are retained):

https://d26abb2-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1132/head:pr_1132 && git checkout pr_1132
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.2.0-d26abb2

markmandel · 2019-12-12T23:46:30Z

@roberthbailey anything left on this before we can merge?

roberthbailey · 2019-12-13T07:50:54Z

site/content/en/docs/Installation/helm.md

@@ -36,8 +36,12 @@ _We recommend to install Agones in its own namespaces (like `agones-system` as s
 you can use the helm `--namespace` parameter to specify a different namespace._

 When running in production, Agones should be scheduled on a dedicated pool of nodes, distinct from where Game Servers are scheduled for better isolation and resiliency. By default Agones prefers to be scheduled on nodes labeled with `agones.dev/agones-system=true` and tolerates node taint `agones.dev/agones-system=true:NoExecute`. If no dedicated nodes are available, Agones will
+<<<<<<< HEAD


Need to resolve this merge conflict.

Should be resolved.

@roberthbailey is this good to go now?

(I'm on a push to close out PRs before EOY 😃)

agones-bot · 2019-12-14T19:22:05Z

Build Succeeded 👏

Build Id: 60d0cac5-3a5c-4a1d-bc09-5129a6b6aace

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:1.2.0-3d31a4a
image: gcr.io/agones-images/agones-sdk:1.2.0-3d31a4a
image: gcr.io/agones-images/agones-ping:1.2.0-3d31a4a
Linux C++ SDK (build): agonessdk-1.2.0-3d31a4a-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.2.0-3d31a4a.zip

A preview of the website (the last 30 builds are retained):

https://3d31a4a-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1132/head:pr_1132 && git checkout pr_1132
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.2.0-3d31a4a

roberthbailey · 2019-12-19T08:55:20Z

pkg/metrics/exporter.go

+func RegisterStackdriverExporter(projectID string, defaultLabels string) (*stackdriver.Exporter, error) {
+	monitoredRes, err := getMonitoredResource(projectID)
+	if err != nil {
+		logger.WithError(err).Warn("error discovering monitored resource")


Is it ok if monitoredRes is nil later on or does this need to be followed by a return nil, err line?

no it's ok this is a best effort.

by default it is nil.

google-oss-robot · 2019-12-19T16:46:12Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cyriltovena, roberthbailey

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [cyriltovena,roberthbailey]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>

Adds more tests for parsing labels. Skip monitored resource if discovery fails.

google-oss-robot · 2019-12-19T16:47:40Z

New changes are detected. LGTM label has been removed.

cyriltovena · 2019-12-19T16:48:54Z

New changes are detected. LGTM label has been removed.

just rebased buddy ! :)

agones-bot · 2019-12-19T17:01:10Z

Build Succeeded 👏

Build Id: da6ee4b3-f53c-4c8f-90df-ee5417cfc9f9

The following development artifacts have been built, and will exist for the next 30 days:

image: gcr.io/agones-images/agones-controller:1.3.0-af1da11
image: gcr.io/agones-images/agones-sdk:1.3.0-af1da11
image: gcr.io/agones-images/agones-ping:1.3.0-af1da11
Linux C++ SDK (build): agonessdk-1.3.0-af1da11-linux-arch_64.tar.gz
SDK Server: agonessdk-server-1.3.0-af1da11.zip

A preview of the website (the last 30 builds are retained):

https://af1da11-dot-preview-dot-agones-images.appspot.com/

To install this version:

git fetch https://github.com/GoogleCloudPlatform/agones.git pull/1132/head:pr_1132 && git checkout pr_1132
helm install ./install/helm/agones --namespace agones-system --name agones --set agones.image.tag=1.3.0-af1da11

* improve stackdriver metric type Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>

google-oss-robot requested review from ilkercelikyilmaz and roberthbailey October 22, 2019 18:37

google-oss-robot added approved size/M labels Oct 22, 2019

markmandel added feature-freeze-do-not-merge Only eligible to be merged once we are out of feature freeze (next full release) kind/feature New features for Agones area/operations Installation, updating, metrics etc labels Oct 22, 2019

roberthbailey reviewed Oct 28, 2019

View reviewed changes

pkg/metrics/exporter.go Outdated Show resolved Hide resolved

pkg/metrics/exporter.go Show resolved Hide resolved

google-oss-robot assigned roberthbailey Oct 28, 2019

markmandel removed the feature-freeze-do-not-merge Only eligible to be merged once we are out of feature freeze (next full release) label Oct 29, 2019

google-oss-robot added size/L and removed size/M labels Nov 6, 2019

ano12ak reviewed Nov 6, 2019

View reviewed changes

pkg/metrics/exporter.go Outdated Show resolved Hide resolved

roberthbailey reviewed Nov 8, 2019

View reviewed changes

markmandel added the feature-freeze-do-not-merge Only eligible to be merged once we are out of feature freeze (next full release) label Dec 4, 2019

cyriltovena force-pushed the stackdriver-res branch from ae192b3 to d26abb2 Compare December 8, 2019 20:50

markmandel removed the feature-freeze-do-not-merge Only eligible to be merged once we are out of feature freeze (next full release) label Dec 12, 2019

roberthbailey reviewed Dec 13, 2019

View reviewed changes

roberthbailey reviewed Dec 19, 2019

View reviewed changes

roberthbailey approved these changes Dec 19, 2019

View reviewed changes

google-oss-robot added the lgtm label Dec 19, 2019

cyriltovena added 9 commits December 19, 2019 11:47

improve stackdriver metric type

9f0a49a

Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>

removed naked return

1283190

Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>

adds a parse label function.

ec19f45

Add some tests

584ac51

hook configs

1691811

add documentation

97e9e17

change metric type

d6de02f

Review feedback.

e5e1de2

Adds more tests for parsing labels. Skip monitored resource if discovery fails.

Fix automatic merge from github.

af1da11

cyriltovena force-pushed the stackdriver-res branch from 3d31a4a to af1da11 Compare December 19, 2019 16:47

google-oss-robot removed the lgtm label Dec 19, 2019

roberthbailey merged commit 64ceb4d into googleforgames:master Dec 19, 2019

markmandel added this to the 1.3.0 milestone Jan 10, 2020

ilkercelikyilmaz pushed a commit to ilkercelikyilmaz/agones that referenced this pull request Oct 23, 2020

improve stackdriver metric type (googleforgames#1132)

8bc8060

* improve stackdriver metric type Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

improve stackdriver metric type #1132

improve stackdriver metric type #1132

cyriltovena commented Oct 22, 2019

agones-bot commented Oct 22, 2019

cyriltovena commented Oct 23, 2019

markmandel commented Oct 23, 2019

roberthbailey commented Oct 23, 2019

roberthbailey commented Oct 28, 2019

agones-bot commented Oct 29, 2019

agones-bot commented Nov 6, 2019

agones-bot commented Nov 6, 2019

agones-bot commented Nov 6, 2019

agones-bot commented Nov 6, 2019

roberthbailey commented Nov 7, 2019

cyriltovena commented Nov 7, 2019

roberthbailey Nov 8, 2019

roberthbailey Nov 8, 2019

roberthbailey Nov 8, 2019

roberthbailey Nov 8, 2019

agones-bot commented Dec 8, 2019

markmandel commented Dec 8, 2019

agones-bot commented Dec 8, 2019

markmandel commented Dec 12, 2019

roberthbailey Dec 13, 2019

cyriltovena Dec 14, 2019

markmandel Dec 18, 2019

agones-bot commented Dec 14, 2019

roberthbailey Dec 19, 2019

cyriltovena Dec 19, 2019

cyriltovena Dec 19, 2019

google-oss-robot commented Dec 19, 2019

google-oss-robot commented Dec 19, 2019

cyriltovena commented Dec 19, 2019

agones-bot commented Dec 19, 2019

improve stackdriver metric type #1132

improve stackdriver metric type #1132

Conversation

cyriltovena commented Oct 22, 2019

agones-bot commented Oct 22, 2019

cyriltovena commented Oct 23, 2019

markmandel commented Oct 23, 2019

roberthbailey commented Oct 23, 2019

roberthbailey commented Oct 28, 2019

agones-bot commented Oct 29, 2019

agones-bot commented Nov 6, 2019

agones-bot commented Nov 6, 2019

agones-bot commented Nov 6, 2019

agones-bot commented Nov 6, 2019

roberthbailey commented Nov 7, 2019

cyriltovena commented Nov 7, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agones-bot commented Dec 8, 2019

markmandel commented Dec 8, 2019

agones-bot commented Dec 8, 2019

markmandel commented Dec 12, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agones-bot commented Dec 14, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

google-oss-robot commented Dec 19, 2019

google-oss-robot commented Dec 19, 2019

cyriltovena commented Dec 19, 2019

agones-bot commented Dec 19, 2019