Skip to content
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.

Sign up for GitHub

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

Enable pprof for elastic-agent and beats #28983

Merged
merged 2 commits into from
Nov 22, 2021
Merged

Enable pprof for elastic-agent and beats #28983

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ac9ebe9
Enable pprof for elastic-agent and beats
michel-laterman Nov 16, 2021
6591a82
Merge remote-tracking branch 'origin/master' into agent-enable-pprof
michel-laterman Nov 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions x-pack/elastic-agent/CHANGELOG.next.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,3 +149,4 @@
- Add diagnostics command to gather beat metadata. {pull}28265[28265]
- Add diagnostics collect command to gather beat metadata, config, policy, and logs and bundle it into an archive. {pull}28461[28461]
- Add `KIBANA_FLEET_SERVICE_TOKEN` to Elastic Agent container. {pull}28096[28096]
- Enable pprof endpoints for beats processes. Allow pprof endpoints for elastic-agent if enabled. {pull}28983[28983]
3 changes: 3 additions & 0 deletions x-pack/elastic-agent/_meta/config/common.p2.yml.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ inputs:
# logs: true
# # enables metrics monitoring
# metrics: true
# # exposes /debug/pprof/ endpoints
# # recommended that these endpoints are only enabled if the monitoring endpoint is set to localhost
# pprof: true
# # exposes agent metrics using http, by default sockets and named pipes are used
# http:
# # enables http endpoint
Expand Down
3 changes: 3 additions & 0 deletions x-pack/elastic-agent/_meta/config/common.reference.p2.yml.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,9 @@ inputs:
# logs: false
# # enables metrics monitoring
# metrics: false
# # exposes /debug/pprof/ endpoints
# # recommended that these endpoints are only enabled if the monitoring endpoint is set to localhost
# pprof: true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is pprof: true while logs and metrics are false by default?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the reference here may be incorrect, the default MonitoringConfig has them set to true

# # exposes agent metrics using http, by default sockets and named pipes are used
# http:
# # enables http endpoint
Expand Down
3 changes: 3 additions & 0 deletions x-pack/elastic-agent/_meta/config/elastic-agent.docker.yml.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,9 @@ inputs:
# logs: false
# # enables metrics monitoring
# metrics: false
# # exposes /debug/pprof/ endpoints
# # recommended that these endpoints are only enabled if the monitoring endpoint is set to localhost
# pprof: true
# # exposes agent metrics using http, by default sockets and named pipes are used
# http:
# # enables http endpoint
Expand Down
3 changes: 3 additions & 0 deletions x-pack/elastic-agent/elastic-agent.docker.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,9 @@ inputs:
# logs: false
# # enables metrics monitoring
# metrics: false
# # exposes /debug/pprof/ endpoints
# # recommended that these endpoints are only enabled if the monitoring endpoint is set to localhost
# pprof: true
# # exposes agent metrics using http, by default sockets and named pipes are used
# http:
# # enables http endpoint
Expand Down
3 changes: 3 additions & 0 deletions x-pack/elastic-agent/elastic-agent.reference.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,9 @@ inputs:
# logs: false
# # enables metrics monitoring
# metrics: false
# # exposes /debug/pprof/ endpoints
# # recommended that these endpoints are only enabled if the monitoring endpoint is set to localhost
# pprof: true
# # exposes agent metrics using http, by default sockets and named pipes are used
# http:
# # enables http endpoint
Expand Down
3 changes: 3 additions & 0 deletions x-pack/elastic-agent/elastic-agent.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ inputs:
# logs: true
# # enables metrics monitoring
# metrics: true
# # exposes /debug/pprof/ endpoints
# # recommended that these endpoints are only enabled if the monitoring endpoint is set to localhost
# pprof: true
# # exposes agent metrics using http, by default sockets and named pipes are used
# http:
# # enables http endpoint
Expand Down
4 changes: 4 additions & 0 deletions x-pack/elastic-agent/pkg/agent/cmd/run.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,10 @@ func setupMetrics(agentInfo *info.AgentInfo, logger *logger.Logger, operatingSys
}
s.Start()

if cfg.Pprof {
s.AttachPprof()
}

// return server stopper
return s.Stop, nil
}
Expand Down
1 change: 1 addition & 0 deletions x-pack/elastic-agent/pkg/core/monitoring/beats/beats_monitor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ func (b *Monitor) EnrichArgs(spec program.Spec, pipelineID string, args []string
appendix = append(appendix,
"-E", "http.enabled=true",
"-E", "http.host="+endpoint,
"-E", "http.pprof.enabled=true",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If monitoring is enabled we always enable also pprof?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes.
Also as far as my testing shows, even when the agent is not monitoring its beats the beats will bind to a socket (and pprof will be available)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this might be unexpected from a user perspective. pprof exposes quite a bit more information than monitoring as it exposes some of the internals of the process. At the same time, whoever gets the monitoring data likely already knows about the internals.

@simitt @scunningham Any concerns with this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you help me understand how the current config options play together:

agent.monitoring:
  enabled: false
  logs: false
  metrics: false
  http:
    enabled: true

Will metrics be exposed via the http endpoint?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not enable pprof unless customer explicitly turns it on. The HTTP interface is not defended and an attacker would be able to potentially steal secrets from the heap dump.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

http.pprof.enabled is a libbeat setting that we are passing to the beats to expose the pprof options over the http endpoint.
When the agent passes it above, the http interface is bound to a local unix socket (or windows npipe)

@scunningham, when the elastic-agent starts a beat, the beat will bind the interface to a socket even if agent.monitoring.enabled: false, is your request to disable automatically enabling the pprof endpoints for these beats unless explicitly enabled in this case as well?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you saying we automatically enable pprof even if not explicitly enabled? That's not great.

)
}

Expand Down
2 changes: 2 additions & 0 deletions x-pack/elastic-agent/pkg/core/monitoring/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ type MonitoringConfig struct {
LogMetrics bool `yaml:"-" config:"-"`
HTTP *MonitoringHTTPConfig `yaml:"http" config:"http"`
Namespace string `yaml:"namespace" config:"namespace"`
Pprof bool `yaml:"pprof" config:"pprof"`
}

// MonitoringHTTPConfig is a config defining HTTP endpoint published by agent
Expand All @@ -38,5 +39,6 @@ func DefaultConfig() *MonitoringConfig {
Port: defaultPort,
},
Namespace: defaultNamespace,
Pprof: true,
}
}