feat: Add APM to DatadogInstrumentation CRD#2984
Conversation
betterengineering
added
enhancement
github-actions
Bot
added
team/container-autoscaling
and removed
team/injection-platform
labels
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ed1d9455e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| config: | ||
| description: Config defines the Datadog instrumentation configuration to apply to the target workload. | ||
| properties: | ||
| apm: |
There was a problem hiding this comment.
This adds spec.config.apm to the generated CRD under config/crd, but the stored OLM bundle CRD was not regenerated: bundle/manifests/datadoghq.com_datadoginstrumentations.yaml still has only config.properties.checks and no apm/ddTraceConfigs entry (verified with rg ddTraceConfigs bundle/manifests/datadoghq.com_datadoginstrumentations.yaml). Because bundle.Dockerfile copies bundle/manifests into the published bundle, installations through the operator bundle will install the old CRD and Kubernetes will prune spec.config.apm, so the Cluster Agent will never see the APM settings for those users.
Useful? React with 👍 / 👎.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2984 +/- ##
==========================================
- Coverage 40.76% 40.74% -0.03%
==========================================
Files 332 332
Lines 28198 28274 +76
==========================================
+ Hits 11495 11520 +25
- Misses 15928 15979 +51
Partials 775 775
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
|
🎯 Code Coverage (details) 🔗 Commit SHA: 5605095 | Docs | Datadog PR Page | Give us feedback! |
| type DatadogInstrumentationAPMConfig struct { | ||
| // Enabled turns on APM via Single Step Instrumentation to automatically install the Datadog SDKs for supported | ||
| // languages with no additional configuration required. | ||
| Enabled bool `json:"enabled,omitempty"` |
There was a problem hiding this comment.
Is an enabled section needed? The APM config already implies this based if the section is added (enabled) or removed (disabled).
There was a problem hiding this comment.
The other two fields are optional and have default values. So I thought it would be weird if we did something like:
config:
apm: {}The current design looks like this, and it's what most users would need to do:
config:
apm:
enabled: trueI agree though, this case feels redundent:
config:
apm:
enabled: true
ddTraceVersions:
python: "3"Do you think there is a way around this?
There was a problem hiding this comment.
Ahh I see. I can't think of anything better to be honest and apm: {} is definitely not the right choice. I'm good to keep enabled. 👍🏽
1ed1d94 to
3d07f44
Compare
This commit adds the schema required to utilize the DatadogInstrumentation CRD for APM.
3d07f44 to
5605095
Compare
gh-worker-dd-devflow-36fce6
Bot
added
mergequeue-status: done
and removed
mergequeue-status: in_progress
labels
This commit adds the schema required to utilize the DatadogInstrumentation CRD for APM. (cherry picked from commit d97a3dc) Co-authored-by: Mark Spicer <[email protected]> Co-authored-by: Mathew Estafanous <[email protected]>
This reverts commit d97a3dc.
3 participants
What does this PR do?
This commit adds the schema required to utilize the
DatadogInstrumentationCRD for APM. Relates to #2962Example
Motivation
Two relevant RFCs:
Additional Notes
All this adds is the schema. The consumer of this schema is the Datadog Cluster Agent.
Describe your test plan
Ran:
Also verified
make generate && make manifestsproduces no additional diff after generated files are up to date.Checklist
bug,enhancement,refactoring,documentation,tooling, and/ordependenciesqa/skip-qalabel