chore: rebase upstream #133
Conversation
# Conflicts: # ebpftracer/ebpf.go
ebpf: improve protocol identification accuracy
instance metadata: add support for Alibaba Cloud
Add version flag and installation of specific version
add support for hybrid cgroup mode with both V1 and V2 enabled
instance metadata: add support for Scaleway
application types: add OpenSearch and VictoriaMetrics
containers: invalidate ignored containers cache
group connections to AWS services by FQDN instead of IPs
application types: add Coroot
enhance accuracy of cgroup2 base path detection
…ation tls: avoid reading the entire `.text` section to reduce memory allocations (#175)
fix detection of actual destination for TCP connections
using FQDNs as destinations when they are resolved into a list of public IPs
logparser v1.1.9
better application type detection
fix NsPid parsing
allow excluding specific HTTP paths from metrics (e.g, health checks)
improve cgroup detection logic for mixed v1 and v2
allow excluding specific HTTP requests from metrics and traces (e.g, health checks)
add support for container allow and deny lists
metadata: add support for IBM cloud
metrics: add config option for max label value length
fix handling of cloud metadata overrides
fix memory leak in HTTP/2 parser
…sing improve error handling in ClickHouse protocol parser
There was a problem hiding this comment.
Hello @mayankpande88, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
Hello team,
Gemini here, providing a summary for this pull request titled "chore: rebase upstream". As the title suggests, the primary goal of this PR is to rebase the current branch against the upstream repository, incorporating the latest changes and updating dependencies. Beyond just syncing, this rebase brings in several notable enhancements and fixes, including improved cgroup handling, new filtering capabilities for containers and HTTP requests, expanded cloud metadata detection, and initial support for JVM perfmap dumping for profiling.
This is a substantial update, touching various core components of the agent. Please review the changes carefully.
Highlights
- Dependency Updates: The pull request includes significant updates to various dependencies, including the Go version (to 1.23.8), Cilium/ebpf libraries, Docker, Prometheus client and common libraries, Kubernetes API and client libraries, and other minor dependencies. This brings the project up-to-date with newer versions of key components.
- Improved Cgroup Handling: The cgroup handling logic has been refactored to better support both v1 and v2. The explicit
Versionfield has been removed from theCgroupstruct, and the code now determines the cgroup version and relevant paths based on the presence of specific subsystems in/proc/<pid>/cgroup. Support for thereserved.slicein systemd cgroups has been added. The logic for finding the cgroup v2 root (cg2Root) has been improved by checking/proc/self/mounts. - Container Filtering: New flags
--container-allowlistand--container-denylisthave been introduced to allow users to filter which containers the agent processes based on regex patterns applied to the container ID. This is implemented via a newcontainerFilterin thecommonpackage. - HTTP Path Filtering: A new flag
--exclude-http-requests-by-pathhas been added to allow users to exclude HTTP metrics and traces based on glob patterns matching the request path. This is implemented via a newhttpFilterin thecommonpackage. - Expanded Cloud Metadata Detection: The agent can now detect and collect instance metadata for Alibaba Cloud, IBM Cloud, and Scaleway, in addition to existing providers. The node collector also now allows overriding detected metadata fields (Provider, Region, AZ, InstanceType, LifeCycle) via command-line flags.
- JVM Perfmap Dumping for Profiling: Initial support for dumping JVM perfmaps has been added. This allows the eBPF profiler to resolve Java symbols. A new environment variable
COROOT_EBPF_PROFILING=disabledcan be used to disable eBPF profiling for specific processes. The agent checks if a JVM process supports perfmap dumping based on command-line flags (-XX:+PreserveFramePointer) before attempting to dump. - New Application Type Detection: The agent can now identify more application types based on process command lines or executable paths, including various Coroot components (
coroot,coroot-ee,coroot-node-agent,coroot-cluster-agent,coroot-operator), OpenSearch, VictoriaMetrics components (victoria-metrics,vmstorage,vminsert,vmselect), VictoriaLogs, and Ruby applications. - Documentation and Install Script Updates: Documentation links in the README have been updated to use
docs.coroot.com. The install script now supports specifying a version to install via-v/--versionand includes a help option (-h/--help).
Changelog
Click here to see the changelog
- Dockerfile
- Updated Go builder image to 1.23.8-bullseye.
- Modified
-ldflagsformat for setting the version.
- README.md
- Updated documentation links from
coroot.com/docstodocs.coroot.com. - Updated the live demo link from
community-demo.coroot.comtodemo.coroot.com.
- Updated documentation links from
- cgroup/cgroup.go
- Added
cg2Rootvariable for cgroup v2 root path. - Removed
Versiontype and field fromCgroupstruct. - Added
getId()method toCgroupfor unified cgroup path resolution. - Modified
CreatedAt()to usecg2Rootfor v2 paths and handle missing subsystem paths. - Updated
NewFromProcessCgroupFileto usegetId()and ignore root/init.scope paths. - Added
reserved.slicetosystemSliceIdRegexpandcontainerByCgroup.
- Added
- cgroup/cgroup_linux.go
- Improved
Init()to detect cgroup v2 root by checking forunifieddirectory and parsing/proc/self/mounts. - Adjusted
Setnslogic for processes not in the host network namespace.
- Improved
- cgroup/cgroup_test.go
- Removed assertions related to the removed
Versionfield. - Added test cases for new cgroup paths and types (e.g., crio, reserved.slice).
- Removed assertions related to the removed
- cgroup/cpu.go
- Modified
CpuStat()to determine cgroup version based on subsystem presence. - Updated
cpuStatV1andcpuStatV2to check for empty subsystem paths and usecg2Rootfor v2.
- Modified
- cgroup/cpu_test.go
- Added
cg2Rootinitialization for tests. - Added test case for cgroup v2 CPU stats.
- Added
- cgroup/fixtures/cgroup/memory/system.slice/ssh.service/memory.limit_in_bytes
- Added fixture file for cgroup v1 memory limit.
- cgroup/fixtures/cgroup/memory/system.slice/ssh.service/memory.stat
- Added fixture file for cgroup v1 memory stats.
- cgroup/fixtures/cgroup/unified/system.slice/ssh.service/cpu.stat
- Added fixture file for cgroup v2 CPU stats.
- cgroup/fixtures/proc/2000/cgroup
- Added fixture file for a cgroup v1 /proc/cgroup entry.
- cgroup/fixtures/proc/550/cgroup
- Added fixture file for a cgroup v2 /proc/cgroup entry.
- cgroup/io.go
- Replaced
ioutil.ReadFilewithos.ReadFile. - Modified
IOStat()to determine cgroup version based on subsystem presence. - Updated
ioStatV1andioStatV2to check for empty subsystem paths and usecg2Rootfor v2.
- Replaced
- cgroup/io_test.go
- Added
cg2Rootinitialization for tests.
- Added
- cgroup/memory.go
- Modified
MemoryStat()to determine cgroup version based on subsystem presence. - Updated
memoryStatV1andmemoryStatV2to check for empty subsystem paths and usecg2Rootfor v2.
- Modified
- cgroup/memory_test.go
- Added
cg2Rootinitialization for tests. - Added test case for cgroup v2 memory stats.
- Added
- common/container.go
- Added
containerFiltertype and related functions for filtering containers by allow/deny lists. - Initialized
ContainerFilterusing new flags.
- Added
- common/container_test.go
- Added tests for
containerFilter.
- Added tests for
- common/net.go
- Imported
glob. - Added
httpFiltertype and related functions for filtering HTTP requests by path. - Initialized
HttpFilterusing a new flag. - Added
IsIpExternalhelper function. - Introduced
Domainstruct andNewDomainfunction to represent resolved domains and decide whether to specify IP in destination keys. - Modified
NewDestinationKeyto accept*Domainand use itsSpecifyIPfield. - Changed
klog.Fatalftoklog.Exitffor flag parsing errors.
- Imported
- common/net_test.go
- Updated
TestDestinationKeyto use the newDomainstruct. - Added
TestDomainfor testing the newDomainlogic.
- Updated
- common/strings.go
- Added
TruncateUtf8function for UTF-8 safe string truncation.
- Added
- common/strings_test.go
- Added tests for
TruncateUtf8.
- Added tests for
- containers/app.go
- Renamed
guessApplicationTypetoguessApplicationTypeByCmdline. - Added new application types (Coroot components, OpenSearch, VictoriaMetrics/Logs, Ruby) to
guessApplicationTypeByCmdline. - Added
guessApplicationTypeByExefunction to guess app type from executable path.
- Renamed
- containers/cilium.go
- Updated bpf map paths to use
defaults.BPFFSRootanddefaults.TCGlobalsPath. - Added
ciliumMapDefinitionandciliumMapsfor better map handling. - Updated
lookupCilium4andlookupCilium6to usee.BackendIDfor backend ID.
- Updated bpf map paths to use
- containers/container.go
- Added
appIdfield toContainerstruct. - Populated
appIdusingcommon.ContainerIdToOtelServiceNameand new app guessing logic. - Truncated log sample labels using
common.TruncateUtf8and a new flagMaxLabelLength. - Updated
onL7Requestto use the newDomainstruct for FQDNs.
- Added
- containers/dotnet.go
- Removed unused import
errors. - Corrected
connectto useproc.GetNsPid. - Modified
dotNetAppto also check cmdline for.dllfiles and removed redundantproc.GetCmdlinecall.
- Removed unused import
- containers/jvm.go
- Removed unused import
bytes. - Modified
jvmMetricsto useproc.GetNsPidand handle errors.
- Removed unused import
- containers/process.go
- Modified
instrumentto get cmdline once and pass it to Python and DotNet instrumentation. - Updated calls to
dotNetAppandinstrumentPythonto accept cmdline. - Changed
isJvmcall to use the newproc.IsJvm.
- Modified
- containers/registry.go
- Added
IgnoredContainersCacheTTLconstant andcontainersByPidIgnoredmap for caching ignored PIDs. - Modified
NewRegistryto initialize the new map and includeapp_idin Prometheus registration labels. - Modified
handleEventsto clear the ignored cache periodically and update Prometheus unregistration labels. - Updated
getOrCreateContainerto check the ignored cache, apply the newcommon.ContainerFilter, and update the ignored cache. - Changed
ip2fqdnmap value type to*common.Domain. - Updated
Collectto only exportIp2FqdnifDomain.SpecifyIPis true. - Renamed
getFQDNtogetDomain.
- Added
- ebpftracer/ebpf/l7/l7.c
- Moved Kafka request check later in
trace_enter_write.
- Moved Kafka request check later in
- ebpftracer/ebpf/tcp/conntrack.c
- Removed checks that skipped events if original destination IP matched reply source IP.
- ebpftracer/l7/clickhouse.go
- Added validation for protocol version, stage, and compression in
ParseClickhouse.
- Added validation for protocol version, stage, and compression in
- ebpftracer/l7/http2.go
- Refactored HTTP/2 parser to initialize decoder outside the loop and use
defer Close(). - Removed redundant decoder assignments inside the loop.
- Refactored HTTP/2 parser to initialize decoder outside the loop and use
- ebpftracer/l7/l7_test.go
- Added test case for ClickHouse
ClientQuerySecondary.
- Added test case for ClickHouse
- ebpftracer/python.go
- Added
link.UprobeOptions{PID: int(pid)}when attaching Python uprobes.
- Added
- ebpftracer/tls.go
- Imported
io. - Changed how text section data is read in
AttachGoTlsUprobesto useio.Reader.
- Imported
- flags/flags.go
- Imported
fmt. - Added new flags:
ContainerAllowlist,ContainerDenylist,ExcludeHTTPMetricsByPath,MaxLabelLength,agentVersion,Version. - Added logic to print version and exit if
agentVersionflag is set. - Changed
klog.Fatalftoklog.Exitffor flag parsing errors.
- Imported
- go.mod
- Updated Go version to 1.23.8.
- Updated numerous dependencies (cilium/ebpf, docker/docker, prometheus, k8s.io libraries, etc.).
- Added
github.com/gobwas/glob. - Added replace directive for
github.com/grafana/pyroscope/ebpf.
- install.sh
- Set default
VERSIONto "latest". - Added
-v/--versionand-h/--helpoptions. - Modified
get_release_versionto use the specified version if provided.
- Set default
- jvm/jattach.go
- New file: Implements logic for attaching to JVM processes via
.attach_pidand.java_pidsocket.
- New file: Implements logic for attaching to JVM processes via
- jvm/perfmap.go
- New file: Provides high-level functions for dumping JVM perfmaps and checking if perfmap dump is supported.
- main.go
- Used
flags.Versionfor the agent version. - Updated Prometheus registration labels to include cloud metadata (region, az) if available from the node collector.
- Used
- node/collector.go
- Modified
NewCollectorto allow overriding detected cloud metadata via flags. - Added
Metadata()method to expose collected metadata. - Updated
Collectto use the storedinstanceMetadata.
- Modified
- node/metadata/alibaba.go
- New file: Added Alibaba Cloud metadata detection.
- node/metadata/ibm.go
- New file: Added IBM Cloud metadata detection, including handling of host network namespace for token retrieval.
- node/metadata/metadata.go
- Added new cloud providers (Alibaba, Scaleway, IBM) to
CloudProviderenum. - Updated
getCloudProviderto detect new providers based on/sys/class/dmi/id/sys_vendorand/sys/class/dmi/id/chassis_vendor. - Updated
GetInstanceMetadatato call getter functions for new providers.
- Added new cloud providers (Alibaba, Scaleway, IBM) to
- node/metadata/scaleway.go
- New file: Added Scaleway metadata detection.
- proc/fixtures/123/status
- Added fixture file for /proc/123/status.
- proc/fixtures/88451/status
- Added fixture file for /proc/88451/status.
- proc/flags.go
- New file: Implements
GetFlagsto read and parseCOROOT_environment variables from/proc/<pid>/environ.
- New file: Implements
- proc/jvm.go
- New file: Moved
IsJvmfunction here fromcontainers/jvm.go.
- New file: Moved
- proc/proc.go
- Imported
errors,fmt. - Modified
GetNsPidto return an error and handle differentNSpidformats. - Added
IsNotExisthelper function.
- Imported
- proc/proc_test.go
- Imported
sort. - Updated
TestListPidsto sort results and include new fixture PID. - Added
TestGetNsPid.
- Imported
- profiling/profiling.go
- Imported
jvm,proc. - Set
PythonFullFilePathto true inSymbolOptions. - Changed
targetFinder.nowtype toint64and storedUnixNano(). - Modified
FindTargetto check process flags (COROOT_EBPF_PROFILING), check JVM perfmap support, and trigger perfmap dumps. - Added new fields (
initialized,flags,jvmPerfmapDumpSupported,lastPerfmapDump) toprocessInfo.
- Imported
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Code changes flow,
Dependencies start to grow,
Rebase, sync, and go.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request, titled "chore: rebase upstream", incorporates a significant number of enhancements, new features, and bug fixes beyond a typical rebase. The changes are generally well-implemented and improve the agent's capabilities and robustness.
Key improvements include:
- Critical HTTP/2 Parser Fix: Addressed a resource leak in the HPACK decoder.
- Cgroup Handling: Major refactoring for cgroup v1/v2 detection and path management, making it more robust.
- Cloud Metadata: Expanded support for Alibaba, IBM, and Scaleway, with added flexibility through flag-based overrides for metadata fields.
- Filtering Capabilities: Introduced container allow/deny lists (regex-based) and HTTP path exclusion for metrics (glob-based).
- Profiling Enhancements: Added JVM perfmap dumping support for more detailed Java profiles and enabled full symbol paths for Python.
- Code Quality: Replaced deprecated
ioutilcalls, improved error handling (e.g.,GetNsPid), and updated numerous dependencies including the Go version.
While the changes are largely positive, a few areas warrant discussion or clarification, particularly around the eBPF conntrack logic and the frequency of JVM perfmap dumps. Overall, this is a substantial and valuable update.
Summary of Findings
- Critical HTTP/2 Parser Fix: A resource leak in the HTTP/2 HPACK decoder has been fixed by ensuring
decoder.Close()is called. This is crucial for long-term stability. - eBPF Conntrack Logic Change: Checks comparing original destination IP with reply source IP in the eBPF conntrack code have been removed. Clarification on the impact of this change on NAT detection is advisable.
- Cgroup Handling Refactor: The system for detecting cgroup versions and determining cgroup paths has been significantly refactored, particularly improving cgroup v2 support and removing the explicit
Versionfield. - Expanded Cloud Provider Metadata: Support for detecting instance metadata has been added for Alibaba, IBM, and Scaleway clouds. Additionally, flags now allow overriding or providing metadata fields.
- New Filtering Capabilities: The agent now supports allow/deny lists for containers using regular expressions and exclusion of HTTP metrics/traces based on path glob patterns.
- JVM Profiling Enhancements: Support for dumping JVM perfmap has been added, enabling more detailed profiling for Java applications. This relies on the
-XX:+PreserveFramePointerJVM flag. - Frequency of JVM Perfmap Dumps: The current logic might trigger JVM perfmap dumps too frequently (potentially every profiling interval). This could have performance implications and might need adjustment.
- Ignored PIDs Cache TTL: A 15-second TTL is used for caching PIDs that are ignored. The rationale for this specific duration could be clarified to ensure it's optimal for various scenarios.
Merge Readiness
This pull request introduces many valuable features and a critical bug fix. The code quality is generally high. However, before merging, I recommend discussing the medium-severity concerns raised:
- The implications of the eBPF conntrack logic changes.
- The potential performance impact and necessity of the current frequency for JVM perfmap dumps.
- The rationale behind the 15-second TTL for the ignored PIDs cache.
Addressing or clarifying these points will ensure the stability and efficiency of the new features. As an AI, I am not authorized to approve pull requests; this decision should be made by human reviewers after considering these points.
No description provided.