Add Elasticsearch, Kibana and fluentd to collect, store and view logs. #327

mdemirhan · 2018-03-08T21:41:58Z

Removed the fluentd sidecar container from ela pods
Added a fluentd daemonset with a fixed configuration to collect ela and build container logs.
Added a "dev" option to setup log collection for ela controllers (see telemetry.md for more information)
Refactored the existing configurations into a clearer folder structure.

mattmoor

I left a few comments, but I'd find this a bit easier to review if we broke this down directory by directory.

mattmoor · 2018-03-09T17:51:00Z

config/monitoring/BUILD

+    template = "prometheus-operator/service.yaml",
+)
+
+k8s_objects(


If you'd like, I can show you how to use a Bazel glob() to make this a bit more concise?

mattmoor · 2018-03-09T17:52:02Z

config/monitoring/BUILD

+
+k8s_object(
+    name = "prometheus-operator-service",
+    template = "prometheus-operator/service.yaml",


This file starts to get pretty long. Can we move these rules into BUILD.bazel files in each directory, and they can export their :prometheus-operator k8s_objects group?

Updated PR with individual BUILD files. Thanks!

mattmoor · 2018-03-09T17:58:31Z

docs/telemetry.md

-1. Forward the Grafana server to your machine:
+2. **everything-dev**: This configuration collects everything in (1) plus Elafros controller logs.
+```shell
+bazel run config/monitoring:everything-dev.apply


We should talk sometime about how we release these configs.

mattmoor · 2018-03-09T17:59:11Z

docs/telemetry.md

 ```
+Then open Kibana UI at this [link](http://localhost:8001/api/v1/namespaces/monitoring/services/kibana-logging/proxy/app/kibana) 
+(*it might take a couple of minutes for the proxy to work*). 
+When Kibana is opened the first time, it will ask you to crete an index. Accept the default options as is. As logs get ingested,


typo: crete

mattmoor · 2018-03-09T18:00:41Z

docs/telemetry.md

+## Default metrics
+Following metrics are collected by default:
+* Elafros controller metrics
+* Istio metrics (mixer, envoy and pilot)


We should talk about how we get Build metrics into here.

@imjasonh FYI

Sure thing. I added two sample metrics into ela-controller and I will be more than happy to make the same for build-controller. Let's sync up on this one.

mattmoor · 2018-03-09T18:01:42Z

docs/telemetry.md

+To enable log collection from other containers and destinations, edit fluentd-es-configmap.yaml (search for "fluentd-containers.log" for the starting point). Then run the following:
+```shell
+kubectl delete -f config/monitoring/fluentd/fluentd-es-ds.yaml
+kubectl delete -f config/monitoring/fluentd/fluentd-es-configmap.yaml


Maybe what you want is kubectl replace?

Yup, didn't know such a command existed :-)

mattmoor · 2018-03-09T18:02:00Z

docs/telemetry.md

+
+## Generating logs
+Use glog to write logs in your code. In your container spec, add the following args to redirect the logs to stderr:


link to glog godoc?

Will add. Thanks!

grantr

Thanks @mdemirhan! I have a few questions, but nothing major. I haven't reviewed any of the yaml files for correctness. I trust you've verified they work 😃

grantr · 2018-03-09T17:18:35Z

cmd/ela-autoscaler/main.go

@@ -170,6 +171,7 @@ func handler(w http.ResponseWriter, r *http.Request) {
 }

 func main() {
+	flag.Parse()


Were any flags added or did they already exist?

Command line arguments are already being passed to the executable. This is to ensure that the arguments are passed and not ignored.

grantr · 2018-03-09T17:18:44Z

cmd/ela-queue/main.go

@@ -183,6 +184,7 @@ func handler(w http.ResponseWriter, r *http.Request) {
 }

 func main() {
+	flag.Parse()


Were any flags added or did they already exist?

Command line arguments are already being passed to the executable. This is to ensure that the arguments are passed and not ignored.

grantr · 2018-03-09T17:20:20Z

docs/telemetry.md

 ```
+Then open Kibana UI at this [link](http://localhost:8001/api/v1/namespaces/monitoring/services/kibana-logging/proxy/app/kibana) 
+(*it might take a couple of minutes for the proxy to work*). 
+When Kibana is opened the first time, it will ask you to crete an index. Accept the default options as is. As logs get ingested,


Typo nit: "create an index"

Thanks, I will fix that.

grantr · 2018-03-09T18:06:25Z

sample/helloworld/helloworld.go

@@ -31,6 +35,9 @@ func handler(w http.ResponseWriter, r *http.Request) {
 }

 func main() {
+	flag.Parse()


Is this for glog flags?

This is for all command line flags - glog is one consumer of the command line flags. And if flags are not parsed before any glog calls, glog writes an error message to stderr and that was causing a lot of noise in the logs.

grantr · 2018-03-09T18:08:47Z

pkg/controller/revision/ela_pod.go

@@ -142,10 +128,6 @@ func MakeElaPodSpec(u *v1alpha1.Revision) *corev1.PodSpec {
 				Name:      nginxConfigMapName,
 				ReadOnly:  true,
 			},
-			{
-				MountPath: nginxLogVolumeMountPath,


Do we need to change the nginx config now that its log destination isn't this volume?

I removed fluentd sidecar container that is watching for nginx log volume. That volume is no longer of use and I removed it from the pod definition as well. As the next step, I will remove nginx completely - waiting on Ian for enabling Istio's Envoy sidecar injection.

grantr · 2018-03-09T18:11:16Z

docs/telemetry.md

 ```
+Then open Kibana UI at this [link](http://localhost:8001/api/v1/namespaces/monitoring/services/kibana-logging/proxy/app/kibana) 


Do the internal kibana links work when accessed via this url?

Yes, they should work as long as you have kubectl proxy running.

…ile.

# Conflicts: # pkg/controller/revision/ela_pod.go

mdemirhan · 2018-03-12T15:42:17Z

/retest

mattmoor · 2018-03-12T15:53:15Z

/buildify

mattmoor

thanks for splitting things up. A few comments inline

mattmoor · 2018-03-12T15:54:19Z

config/monitoring/fluentd/BUILD

+load("@k8s_object//:defaults.bzl", "k8s_object")
+
+
+k8s_object(


There is a tool call buildifier that will format these for you. I added a plugin to Prow so that /buildify will report when files aren't properly formed and tell you the command to run them, but I think it's struggling with our private repo :(

I will run the tool. Thanks for pointing that out!

mattmoor · 2018-03-12T15:54:40Z

config/monitoring/fluentd/BUILD

+k8s_object(
+    name = "istio",
+    template = "istio.yaml",
+    visibility = ["//visibility:public"],


Why is this specifically public vs. everything?

mattmoor · 2018-03-12T15:58:52Z

config/monitoring/prometheus-servicemonitor/BUILD

+)
+
+k8s_objects(
+    name = "everything-extended",


Can you add comments to these groupings?

mattmoor · 2018-03-12T16:02:22Z

config/monitoring/prometheus/BUILD

+k8s_object(
+    name = "prometheus-obj",
+    template = "prometheus.yaml",
+    visibility = ["//visibility:public"],


why public?

On a brand new cluster, installing prometheus fails with an error that says "monitoring.coreos.com/v1" not found. I found that installing prometheus after installing service monitors and grafana mitigates this issue. I have yet to find the root cause of this issue (I have seen others hitting this issue with other components and they were asked to open a bug to k8s team - not sure if it is the same).

mdemirhan · 2018-03-12T16:18:46Z

/retest

mattmoor · 2018-03-12T17:23:07Z

config/monitoring/common/BUILD

+    name = "namespace",
+    template = "namespace.yaml",
+    visibility = ["//visibility:public"],
+)


I wonder if this should just move up to config/monitoring?

mattmoor · 2018-03-12T17:24:45Z

config/monitoring/elasticsearch/statefulset.yaml

@@ -0,0 +1,122 @@
+# Copyright 2018 Google LLC


This file has a bit more than a StatefulSet. Perhaps break it apart or rename?

mdemirhan · 2018-03-12T18:04:25Z

/retest

# Conflicts: # pkg/controller/revision/ela_pod.go # sample/helloworld/helloworld.go

mattmoor

I'm eager to try this out :)

mattmoor · 2018-03-12T22:56:41Z

docs/telemetry.md

+## Default logs
+Deployment above enables collection of the following logs:
+* stdout & stderr from all ela-container
+* stdout & stderr from build-controller


Do you really mean build-controller and not the user's build container?

mdemirhan added 2 commits March 8, 2018 13:36

Add Elasticsearch, Kibana and fluentd to collect, store and view logs.

68a1dac

Fixes.

4a306ac

mdemirhan requested review from grantr, mchmarny, mattmoor, vaikas, akyyy and a team March 8, 2018 21:41

google-prow-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Mar 8, 2018

Additional documentation.

fed144e

mattmoor reviewed Mar 9, 2018

View reviewed changes

grantr reviewed Mar 9, 2018

View reviewed changes

mdemirhan added 3 commits March 9, 2018 12:08

Addressing code review comments: Moved each folder to its own BUILD f…

ec47094

…ile.

Merge branch 'master-upstream' into mdemirhan/logging

0234d8a

# Conflicts: # pkg/controller/revision/ela_pod.go

Merge branch 'master-upstream' into mdemirhan/logging

643b8f9

mattmoor reviewed Mar 12, 2018

View reviewed changes

mdemirhan added 3 commits March 12, 2018 11:20

Merge branch 'master-upstream' into mdemirhan/logging

5878111

# Conflicts: # pkg/controller/revision/ela_pod.go # sample/helloworld/helloworld.go

Address PR comments.

c662b41

Double pushing istio rules as a workaround for an istio issue.

effbd63

mattmoor approved these changes Mar 12, 2018

View reviewed changes

Minor changes on the ordering of the installation.

f8a1b35

mdemirhan merged commit cf463ba into knative:master Mar 12, 2018

mdemirhan deleted the mdemirhan/logging branch March 12, 2018 23:03


		## Generating logs
		Use glog to write logs in your code. In your container spec, add the following args to redirect the logs to stderr:

		```
		Then open Kibana UI at this [link](http://localhost:8001/api/v1/namespaces/monitoring/services/kibana-logging/proxy/app/kibana)

		load("@k8s_object//:defaults.bzl", "k8s_object")


		k8s_object(

Add Elasticsearch, Kibana and fluentd to collect, store and view logs. #327

Add Elasticsearch, Kibana and fluentd to collect, store and view logs. #327

Conversation

mdemirhan commented Mar 8, 2018

mattmoor left a comment

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

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

grantr left a comment

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

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

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

Choose a reason for hiding this comment

mdemirhan commented Mar 12, 2018

mattmoor commented Mar 12, 2018

mattmoor left a comment

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

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

Choose a reason for hiding this comment

mdemirhan commented Mar 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mdemirhan commented Mar 12, 2018

mattmoor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment