K8SPSMDB-1546 add PMM querySource#2380
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for selecting the MongoDB Query Analytics (QAN) collection source for PMM by introducing a spec.pmm.querySource field (enum: profiler/mongolog) and wiring it into the PMM sidecar’s pmm-admin add invocation, along with accompanying CRD, unit test, and E2E test updates.
Changes:
- Add
PMMSpec.QuerySourceto the API/CRD and pass it to PMM via--query-source=...when set. - Extend unit tests to verify the
--query-sourceflag is added/omitted correctly. - Update PMM3 E2E scenario to enable logcollector and validate QAN collection after switching to
mongolog.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/psmdb/pmm/pmm.go | Append --query-source to the PMM pmm-admin add command when configured. |
| pkg/psmdb/pmm/pmm_test.go | Add assertions/tests for querySource flag presence/absence. |
| pkg/apis/psmdb/v1/psmdb_types.go | Introduce PMMSpec.QuerySource with kubebuilder enum validation and docs. |
| config/crd/bases/psmdb.percona.com_perconaservermongodbs.yaml | CRD schema update to include pmm.querySource. |
| deploy/crd.yaml | Bundle CRD schema update for pmm.querySource. |
| deploy/bundle.yaml | Bundle CRD schema update for pmm.querySource. |
| deploy/cw-bundle.yaml | Bundle CRD schema update for pmm.querySource. |
| e2e-tests/version-service/conf/crd.yaml | Test CRD schema update for pmm.querySource. |
| deploy/cr.yaml | Document the new pmm.querySource option in the example CR. |
| e2e-tests/monitoring-pmm3/conf/monitoring-pmm3-rs0.yml | Enable logcollector in the PMM3 monitoring E2E config. |
| e2e-tests/monitoring-pmm3/run | Patch cluster during E2E to switch QAN source to mongolog and validate QAN. |
| e2e-tests/monitoring-pmm3/compare/statefulset_monitoring-pmm3-rs0.yml | Update expected manifests to include logcollector env/sidecars. |
| e2e-tests/monitoring-pmm3/compare/statefulset_monitoring-pmm3-rs0-oc.yml | Same as above for OpenShift compare output. |
| e2e-tests/monitoring-pmm3/compare/statefulset_monitoring-pmm3-rs0-no-pmm.yml | Same as above for “no pmm-client container yet” compare output. |
| e2e-tests/monitoring-pmm3/compare/statefulset_monitoring-pmm3-rs0-no-pmm-oc.yml | Same as above for OpenShift “no pmm-client container yet”. |
| e2e-tests/monitoring-pmm3/compare/statefulset_monitoring-pmm3-cfg.yml | Update expected manifests for configsvr with logcollector. |
| e2e-tests/monitoring-pmm3/compare/statefulset_monitoring-pmm3-cfg-oc.yml | Same as above for OpenShift compare output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if cr.Spec.PMM.QuerySource != "" { | ||
| pmmServerArgs += " --query-source=" + cr.Spec.PMM.QuerySource | ||
| } |
There was a problem hiding this comment.
it can't really happen in practice. querySource only exists in the CRD from 1.23, so you need the 1.23 operator to set it. With sequential upgrades, you get to 1.23 from 1.22 — and 1.22 already mounts /data/db (PMM3 gate is >= 1.22.0). So during a normal 1.22 → 1.23 upgrade the mount is always there and mongolog works.
The only way to break it is to manually pin crVersion below 1.22 on a 1.23 operator, which isn't a supported upgrade path.
| # customClusterName: mongo-cluster | ||
| # mongodParams: --environment=ENVIRONMENT | ||
| # mongosParams: --environment=ENVIRONMENT | ||
| # querySource: mongolog |
| if cr.Spec.PMM.QuerySource != "" { | ||
| pmmServerArgs += " --query-source=" + cr.Spec.PMM.QuerySource | ||
| } |
| "pmm enabled - query-source=mongolog is set": { | ||
| secret: tokenSecret, | ||
| setup: func(cr *api.PerconaServerMongoDB) { | ||
| cr.Spec.PMM.QuerySource = "mongolog" | ||
| }, |
| // QuerySource defines the source for Query Analytics (QAN) data collection. | ||
| // Use "profiler" to collect queries via the MongoDB profiler (default), | ||
| // or "mongolog" to collect queries from mongod log files (requires PMM >= 3.3.0 | ||
| // and mongod configured to write logs to /data/db/logs/). |
| yq eval '(.spec | select(.image == null)).image = "'"$IMAGE_MONGOD"'"' "$test_dir/conf/$cluster-rs0.yml" \ | ||
| | yq eval '(.spec | select(has("pmm"))).pmm.image = "'"$IMAGE_PMM3_CLIENT"'"' - \ | ||
| | yq eval '(.spec | select(has("pmm"))).pmm.customClusterName = "'"$custom_cluster_name"'"' - \ | ||
| | yq eval '(.spec | select(has("logcollector"))).logcollector.image = "'"$IMAGE_LOGCOLLECTOR"'"' - \ |
| // QuerySource defines the source for Query Analytics (QAN) data collection. | ||
| // Use "profiler" to collect queries via the MongoDB profiler (default), | ||
| // or "mongolog" to collect queries from mongod log files (requires PMM >= 3.3.0 |
There was a problem hiding this comment.
Noticed that we have maxDescLen=0 while generating CRD, so all these comments (which do a great job explaining how the field should be used) don't end up anywhere.
It's not directly related to this PR, I'm just curious why so. Are there any limits we hit so we need to exclude the descriptions?
| // or "mongolog" to collect queries from mongod log files (requires PMM >= 3.3.0 | ||
| // and mongod configured to write logs to /data/db/logs/). | ||
| // +kubebuilder:validation:Enum=profiler;mongolog | ||
| QuerySource string `json:"querySource,omitempty"` |
There was a problem hiding this comment.
should we configure the default here?
| # customClusterName: mongo-cluster | ||
| # mongodParams: --environment=ENVIRONMENT | ||
| # mongosParams: --environment=ENVIRONMENT | ||
| # querySource: mongolog |
There was a problem hiding this comment.
if profiler is the default, should we have that here?
hors
left a comment
There was a problem hiding this comment.
@nmarukovich please check PMM3 test
| if cr.Spec.PMM.QuerySource != "" { | ||
| pmmServerArgs += " --query-source=" + cr.Spec.PMM.QuerySource | ||
| } |
| "pmm enabled - query-source=mongolog is set": { | ||
| secret: tokenSecret, | ||
| setup: func(cr *api.PerconaServerMongoDB) { | ||
| cr.Spec.PMM.QuerySource = "mongolog" | ||
| }, | ||
| assert: assertQuerySource("mongolog"), | ||
| }, | ||
| "pmm enabled - query-source=profiler is set": { | ||
| secret: tokenSecret, | ||
| setup: func(cr *api.PerconaServerMongoDB) { | ||
| cr.Spec.PMM.QuerySource = "profiler" | ||
| }, | ||
| assert: assertQuerySource("profiler"), | ||
| }, | ||
| "pmm enabled - query-source not set omits flag": { | ||
| secret: tokenSecret, | ||
| assert: assertNoQuerySource(), | ||
| }, | ||
| } |
a46d8e0
| // QuerySource defines the source for Query Analytics (QAN) data collection. | ||
| // Use "profiler" to collect queries via the MongoDB profiler (default), | ||
| // or "mongolog" to collect queries from mongod log files (requires PMM >= 3.3.0 | ||
| // and mongod configured to write logs to /data/db/logs/). |
commit: 5896eaf |
CHANGE DESCRIPTION
Problem:
Add a new pmm.querySource field to the CR that lets users choose how PMM collects Query Analytics (QAN) data:
profiler (default)
mongolog — from mongod log files (requires PMM >= 3.3.0 and logcollector enabled so logs are written at /data/db/logs/)
When set, the value is passed to pmm-admin via --query-source.
Tests:
Add mongolog QAN check to the monitoring-pmm3 e2e test.
Update compare files.
Cause:
Short explanation of the root cause of the issue if applicable.
Solution:
Short explanation of the solution we are providing with this PR.
CHECKLIST
Jira
Needs Doc) and QA (Needs QA)?Tests
compare/*-oc.yml)?Config/Logging/Testability