[Fleet] Enable log introspections by michalpristas · Pull Request #14390 · elastic/beats

michalpristas · 2019-11-05T09:47:37Z

Enables log introspection by introducing monitoring component into application
Flow

FLEET_MONITORING program is injected into AST with
- monitoring configuration from agent spec
- list of programs included in configuration
- FLEET_MONITORING is appended as a last step of configuration change
operator detect FLEET_MONITORING and start/stop respective beats to monitor monitorable beats
beats are made monitorable by
- including logging configuration
- including http.enabled with host specified targeting unix socket

ph · 2019-11-22T20:47:45Z

#14558 was merged in master and I've rebased the fleet branch with the commit.

ph · 2019-11-26T17:09:02Z

Issues on CI doesn't seems related to the changes in this PR.

ok  	github.com/elastic/beats/x-pack/agent/pkg/sorted	1.067s	coverage: 100.0% of statements

=== RUN   TestTokenBucket

--- FAIL: TestTokenBucket (0.13s)

    token_bucket_test.go:39: 

        	Error Trace:	token_bucket_test.go:39

        	Error:      	Not equal: 

        	            	expected: 5

        	            	actual  : 4

        	Test:       	TestTokenBucket

michalpristas · 2019-11-26T17:15:01Z

no it's not related this is timing issue, this test fails on TEST_ENVIRONMENT=0 quite often i did some refactoring of test but timing on flagged env is quite difficult

ph

The strategy looks good, could we add a few more tests concerning the injection and the addition to the operator?

ph · 2019-11-26T17:14:15Z

+	return result
+}
+
+func (o *Operator) handleStopSidecar(s configrequest.Step) error {


NIT: you can also use a named return so you don't have to initialize the variable.

Suggested change

func (o *Operator) handleStopSidecar(s configrequest.Step) error {

func (o *Operator) handleStopSidecar(s configrequest.Step) result error {

ph · 2019-11-26T17:16:51Z

+
+	outputIface, found := config[outputKey]
+	if !found {
+		o.logger.Errorf("operator.getMonitoringSteps: monitoring configuration not found for sidecar: %v", config)


I think this can leak credentials in the log?

very good catch

ph · 2019-11-26T17:17:11Z

+
+	output, found := outputMap["elasticsearch"]
+	if !found {
+		o.logger.Error("operator.getMonitoringSteps: monitoring is missing an elasticsearch configuration from output: %v", outputMap)


Credentials leaks possible?

ph · 2019-11-26T17:21:08Z

 )

-func emitter(log *logger.Logger, router *router) emitterFunc {
+type decoratorFunc = func(*transpiler.AST, []program.Program) ([]program.Program, error)


I like the idea of the decorators.

ph · 2019-11-26T17:34:18Z

+		}
+
+		steps = append(steps, metricbeatStep)
+	}


I wonder if we should extract the "making of the step into two distinct method" so we can test them in isolation.

ph · 2019-11-26T17:49:34Z

+
+// NewMonitor creates a beats monitor.
+func NewMonitor(process, pipelineID string, downloadConfig *artifact.Config, monitorLogs, monitorMetrics bool) *Monitor {
+	var monitoringEndpoint, loggingPath string


LoggingPath is per pipeline id + process, Correct?

path is common for multiple processes, in same directory there are multiple files e.g metricbeat, metricbeat.1... filebeat...name based on process

ph · 2019-11-26T17:50:45Z

+	}
+
+	return append(args, appendix...)
+}


++ for using arguments here for beats, we might have to refactor that in to support endpoint, I hope they adopt the beats way here but I don't know the plan.

ph · 2019-11-26T17:54:56Z

+
+const (
+	// args: pipeline name, application name
+	logFileFormat = "/var/log/%s/%s"


In the above case is the first %s the process? I think we should encapsulate all the logs under an elastic-agent directory.

ph · 2019-11-26T17:55:30Z

+	logFileFormatWin = "%s\\logs\\%s\\%s"
+
+	// args: pipeline name, application name
+	mbEndpointFileFormat = "unix:///var/run/fleet/%s/%s/%s.sock"


s/fleet/elastic-agent

ph · 2019-11-26T17:58:36Z

+
+	// so far we support only beats monitoring
+	return beats.NewMonitor(process, pipelineID, downloadConfig, monitorLogs, monitorMetrics)
+}


Oh I see what you did here, this will work for endpoint too :)

with agent we will either create a new monitor and modify this factory or they will support beats interface and we do nothing. but this allow us to do both

ph · 2019-11-26T18:01:06Z

no it's not related this is timing issue, this test fails on TEST_ENVIRONMENT=0 quite often i did some refactoring of test but timing on flagged env is quite difficult

Let's create an issue to prioritize it.

ph · 2019-12-02T20:54:28Z

OK, code LGTM, I need to actually test/try this.

One thing we didn't consider is how we will monitor the Agent itself? Should we run the processes in a special streams?

michalpristas · 2019-12-03T11:43:22Z

if agent is connected to fleet we have a checkin api which contains subset of logs/events.

log-wise we dont monitor so far. simplest way would be to create a sidecar for an agent or agent sending it directly. which i guess is related to your latest issue

ph · 2019-12-03T18:03:32Z

Yes, we don't, lets create an issue to track it but having a sidecar to monitor the agents seems what we should do. in the end it's just matter of deciding in which streams the sidecards are created and inject the appropriate configuration.

ph · 2019-12-03T18:04:19Z

@michalpristas code LGTM, can we rebase? I've created a conflict because of multiples outputs sorry about that. :(

michalpristas · 2019-12-04T18:01:20Z

this solution creates a monitoring for each output, so streams using output A are monitored by one set of sidecars which then sends logs to output A,
and streams using B output are monitored by different set sending results to B.

another option would be to create

DEFAULT output which will always be there and will be used also for monitoring
monitoring.output option which will use monitoring
^^ these will also mean that we will monitor all streams using single set of monitoring sidecars

cc @ph

ph · 2019-12-04T18:04:55Z

@michalpristas let's merge this as is and do a followup PR, but we will need to actually be able to specify the outputs using monitoring.elasticsearch, From what I've seen in the UI we can only specify a single monitoring per policy. @mattapperson or @nchaulet can confirm that?

nchaulet · 2019-12-04T18:15:32Z

@ph yes you can just configure one monitoring per policy

michalpristas · 2019-12-05T09:05:35Z

i made the change to support monitoring.elasticsearch
look like it does not make sense to have it in a way i had it yesterday it is more complicated solution which does not comply to out contract.

merged PR from fleet

5590fc3

michalpristas added the fleet label Nov 5, 2019

michalpristas added 9 commits November 5, 2019 11:04

missing files oh my

c2e585f

fmt

a812378

conditional append

4e186f8

const for keys

2da406e

outputs

c2dbc62

beat name

70b22eb

beat name

cd1d0d0

data path injection

477a4f8

Merge branch 'fleet' into feat-fleet-logintro

8b21e2d

urso added the Team:Beats label Nov 14, 2019

more readable types

b9be9a6

ph self-assigned this Nov 15, 2019

ph force-pushed the fleet branch from df7daca to 03f1a64 Compare November 18, 2019 20:31

ph removed the Team:Beats label Nov 19, 2019

ph force-pushed the fleet branch from 03f1a64 to ae216b0 Compare November 21, 2019 13:41

ph added the in progress Pull request is currently in progress. label Nov 22, 2019

ph force-pushed the fleet branch from ae216b0 to 9c124fc Compare November 22, 2019 20:47

michalpristas added 2 commits November 25, 2019 14:41

Merge branch 'fleet' into feat-fleet-logintro

82ecda3

http prefix for unix paths

5ee8c78

michalpristas force-pushed the feat-fleet-logintro branch from 15121d2 to 5ee8c78 Compare November 25, 2019 14:19

michalpristas marked this pull request as ready for review November 26, 2019 09:18

ph self-requested a review November 26, 2019 17:09

ph suggested changes Nov 26, 2019

View reviewed changes

resolved comments

26f08d4

michalpristas added 2 commits November 27, 2019 14:40

confglicts

55afb6c

Merge branch 'fleet' into feat-fleet-logintro

37d2408

ph approved these changes Dec 3, 2019

View reviewed changes

ph mentioned this pull request Dec 3, 2019

[Agent] Discuss the monitoring of the Agent (Logs + metrics) #14914

Closed

michalpristas added 3 commits December 4, 2019 12:56

merged conflicts

ad816ea

Merge branch 'fleet' into feat-fleet-logintro

79ead69

groups done right

44da9c1

use monitoring.elasticsearch for output

4a84c25

michalpristas merged commit 1bc5895 into elastic:fleet Dec 5, 2019

leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023

[Fleet] Enable log introspections (elastic#14390)

4aaf1c0

	func (o *Operator) handleStopSidecar(s configrequest.Step) error {
	func (o *Operator) handleStopSidecar(s configrequest.Step) result error {

Conversation

michalpristas commented Nov 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ph commented Nov 22, 2019

Uh oh!

ph commented Nov 26, 2019

Uh oh!

michalpristas commented Nov 26, 2019

Uh oh!

ph left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ph commented Nov 26, 2019

Uh oh!

ph commented Dec 2, 2019

Uh oh!

michalpristas commented Dec 3, 2019

Uh oh!

ph commented Dec 3, 2019

Uh oh!

ph commented Dec 3, 2019

Uh oh!

michalpristas commented Dec 4, 2019

Uh oh!

ph commented Dec 4, 2019

Uh oh!

nchaulet commented Dec 4, 2019

Uh oh!

michalpristas commented Dec 5, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

michalpristas commented Nov 5, 2019 •

edited

Loading