-
Notifications
You must be signed in to change notification settings - Fork 31
feat(docs): improve docs throughout collector #3604
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,17 @@ | ||
| # UBI-micro: required for Red Hat container certification. The smallest UBI | ||
| # variant — no package manager, keeping the final image minimal. | ||
| FROM registry.access.redhat.com/ubi10/ubi-micro:latest AS ubi-micro-base | ||
|
|
||
| # ubi-micro has no dnf, so we use the full UBI image to install packages into | ||
| # an --installroot overlay. The final image gets the packages without dnf. | ||
| FROM registry.access.redhat.com/ubi10/ubi:latest AS package_installer | ||
|
|
||
| # Copy ubi-micro base to /out to preserve its rpmdb | ||
| COPY --from=ubi-micro-base / /out/ | ||
|
|
||
| # Install packages directly to /out/ using --installroot | ||
| # Install runtime dependencies into /out/ using --installroot. | ||
| # elfutils-libelf: BPF ELF parsing; tbb: sinsp runtime dep; curl-minimal: | ||
| # healthcheck; libcap-ng: capability ops; ca-certificates: TLS to Sensor. | ||
| RUN dnf install -y \ | ||
| --installroot=/out/ \ | ||
| --releasever=10 \ | ||
|
|
@@ -15,6 +21,8 @@ RUN dnf install -y \ | |
| dnf clean all --installroot=/out/ && \ | ||
| rm -rf /out/var/cache/dnf /out/var/cache/yum | ||
|
|
||
| # Final image is based on ubi-micro (not package_installer) to exclude dnf | ||
| # and the full UBI toolchain — only the /out/ overlay is merged in. | ||
| FROM ubi-micro-base | ||
|
|
||
| ARG COLLECTOR_VERSION | ||
|
|
@@ -37,12 +45,17 @@ COPY --from=package_installer /out/ / | |
|
|
||
| COPY container/THIRD_PARTY_NOTICES/ /THIRD_PARTY_NOTICES/ | ||
| COPY kernel-modules /kernel-modules | ||
| # self-checks is a separate binary for BPF driver validation at startup, | ||
| # kept independent from the full collector dependency tree. | ||
| COPY container/bin/collector /usr/local/bin/ | ||
|
Comment on lines
+48
to
50
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Correct the
As per path instructions, this is a cross-layer maintainability issue with release-time dependency implications, not a wording nit. 🤖 Prompt for AI AgentsSource: Path instructions |
||
| COPY container/bin/self-checks /usr/local/bin/self-checks | ||
| COPY container/status-check.sh /usr/local/bin/status-check.sh | ||
|
|
||
| # 8080: civetweb introspection (/ready, /state). 9090: Prometheus metrics. | ||
| EXPOSE 8080 9090 | ||
|
|
||
| # Docker-native healthcheck for non-K8s environments and local dev/debugging. | ||
| # 5s start-period avoids false failures while the BPF driver loads. | ||
| HEALTHCHECK \ | ||
| # health checks within the first 5s are not counted as failure | ||
| --start-period=5s \ | ||
|
|
@@ -51,4 +64,6 @@ HEALTHCHECK \ | |
| # the command uses /ready API | ||
| CMD /usr/local/bin/status-check.sh | ||
|
|
||
| # Exec form ensures collector runs as PID 1, receiving signals directly for | ||
| # graceful shutdown of the BPF probe and gRPC connection. | ||
| ENTRYPOINT ["collector"] | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -170,10 +170,19 @@ class ParserYaml { | |||||||||||||||||||||||||||||
| ValidationMode validation_mode_; | ||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||
| * Reload configuration based on inotify events received on a | ||||||||||||||||||||||||||||||
| * configuration file. | ||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||
| /// Watches the runtime configuration file (default: /etc/stackrox/runtime_config.yaml) | ||||||||||||||||||||||||||||||
| /// for changes via inotify and hot-reloads the CollectorConfig accordingly. | ||||||||||||||||||||||||||||||
| /// | ||||||||||||||||||||||||||||||
| /// Uses protobuf reflection to map YAML fields to the sensor::CollectorConfig | ||||||||||||||||||||||||||||||
| /// proto, so adding new config fields only requires updating the proto definition | ||||||||||||||||||||||||||||||
| /// — no code changes needed in the loader itself. | ||||||||||||||||||||||||||||||
| /// | ||||||||||||||||||||||||||||||
| /// Handles both direct files and symlinks (Kubernetes ConfigMaps are mounted | ||||||||||||||||||||||||||||||
| /// as symlink chains that get atomically swapped, which generates different | ||||||||||||||||||||||||||||||
| /// inotify events than a simple file write). Three watchers are maintained: | ||||||||||||||||||||||||||||||
| /// - LOADER_PARENT_PATH: directory-level events (create/delete of the file) | ||||||||||||||||||||||||||||||
| /// - LOADER_CONFIG_FILE: events on the config file/symlink itself | ||||||||||||||||||||||||||||||
| /// - LOADER_CONFIG_REALPATH: events on the symlink target (for ConfigMap swaps) | ||||||||||||||||||||||||||||||
|
Comment on lines
+180
to
+185
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Document these as up to three watcher categories.
Proposed wording-/// as symlink chains that get atomically swapped, which generates different
-/// inotify events than a simple file write). Three watchers are maintained:
+/// as symlink chains that get atomically swapped, which generates different
+/// inotify events than a simple file write). Up to three watcher categories
+/// are used:
...
-/// - LOADER_CONFIG_REALPATH: events on the symlink target (for ConfigMap swaps)
+/// - LOADER_CONFIG_REALPATH: events on the symlink target, when the config
+/// path is a symlink (for ConfigMap swaps)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||
| class ConfigLoader { | ||||||||||||||||||||||||||||||
| public: | ||||||||||||||||||||||||||||||
| ConfigLoader() = delete; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
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.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: stackrox/collector
Length of output: 1860
🏁 Script executed:
Repository: stackrox/collector
Length of output: 3098
Reword this to avoid implying a fully static binary.
BUILD_SHARED_LIBSonly changes the default for project libraries; the image still ships shared runtime deps likelibstdc++,openssl,tbb, andlibelfremains shared. The cache help text should also describe static/shared linkage, not position-independent code.🤖 Prompt for AI Agents
Source: Path instructions