-
Notifications
You must be signed in to change notification settings - Fork 349
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Prometheus scrape annotations to agent #684
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - just one minor formatting issue to resolve.
pkg/inject/sidecar_test.go
Outdated
@@ -160,7 +160,6 @@ func TestInjectSidecarWithEnvVarsOverridePropagation(t *testing.T) { | |||
assert.Contains(t, dep.Spec.Template.Spec.Containers[0].Env, traceContextEnvVar) | |||
assert.Contains(t, dep.Spec.Template.Spec.Containers[0].Env, corev1.EnvVar{Name: envVarServiceName, Value: "testapp.default"}) | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Can you restore this line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
Add default prometheus annotations to be inserted at sidecar in case the deployment don't have any Test(sidecar):Insertion of prometheus annotations Add a test for a case with a deployment without any annotation and another one for a deployment with annotations different from those that we insert as default Signed-off-by: Nivaldo Melo <[email protected]>
The tests are now failing in a recipe for target vendor, that's strange since i only added a newline in a file. Locally i can run |
It failed due to this:
I'm restarting the jobs, hopefully the host is up again. |
CI passed. Thanks for your contribution! |
As discussed in #28, it was created a way to insert prometheus annotations at agents that are deployed as sidecars so it won't overwrite similar annotations provided by the service.
Closes #28