Skip to content

[Feature][issue-423] add datadog profiling to SR Helm #437

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

Merged
merged 4 commits into from
Feb 4, 2024

Conversation

mingmxu
Copy link
Contributor

@mingmxu mingmxu commented Feb 1, 2024

Description

Add datadog_profiling options in Helm chart to simplify the settings to enable dd profiling.

Related Issue(s)

Please list any related issues and link them here.

Checklist

For operator, please complete the following checklist:

  • run make generate to generate the code.
  • run golangci-lint run to check the code style.
  • run make test to run UT.
  • run make manifests to update the yaml files of CRD.

For helm chart, please complete the following checklist:

  • make sure you have updated the values.yaml
    file of starrocks chart.
  • In scripts directory, run bash create-parent-chart-values.sh to update the values.yaml file of the parent
    chart( kube-starrocks chart).

-----Test Plan----

  1. helm lint

  2. helm template with settings on/off, and see the diff of helm template starrocks .
    Screenshot 2024-02-01 at 12 08 35 AM
    Screenshot 2024-02-01 at 12 08 13 AM

  3. apply the new StarRocksCluster yaml

----Revision 2 Test Plan-------

  1. turn on be/fe profiling, no difference compared with revision 1;
Screenshot 2024-02-01 at 2 03 29 PM
  1. Add starrocksBeSpec.capabilities in values to confirm it's merged correctly
  capabilities:
    add:
      - PERFMON
      - SYS_PTRACE
    drop:
      - SYS_ADMIN

See output as below

Screenshot 2024-02-01 at 2 04 11 PM

----revision 4 test plan-----
switch enabledBe/enabledCn: true/false, plus with additional capabilities

  capabilities:
    add:
      - PERFMON
      - SYS_PTRACE
    drop:
      - SYS_ADMIN

The diff of helm template is shown as below:
Screenshot 2024-02-02 at 9 42 41 AM

Dry-run

k apply -n xxxx -f ./issue-423.on.r3.yaml --dry-run="server"
configmap/kube-starrocks-cn-cm created (server dry run)
configmap/kube-starrocks-fe-cm created (server dry run)
starrockscluster.starrocks.com/kube-starrocks created (server dry run)

@mingmxu mingmxu force-pushed the issue-423/datadog-helm-conf branch 2 times, most recently from 073bf18 to 5aa4928 Compare February 1, 2024 08:13
@yandongxiao
Copy link
Collaborator

I would like to ask, what is your screenshot tool?

@mingmxu
Copy link
Contributor Author

mingmxu commented Feb 2, 2024

I would like to ask, what is your screenshot tool?

It's the Compare Files feature in Intellij. If you want to re-produce it, here are the full steps:

cd starrocks-kubernetes-operator/helm-charts/charts/kube-starrocks/charts/starrocks
# 1. do nothing to render default yaml
helm template starrocks . > ./issue-423.off.yaml

# 2. update values.yaml to turn on profiling
helm template starrocks . > ./issue-423.on.yaml

# 3. compare the two files ./issue-423.off.yaml and /issue-423.on.yaml

@mingmxu mingmxu force-pushed the issue-423/datadog-helm-conf branch 2 times, most recently from 54df260 to cc1e56d Compare February 2, 2024 18:00
run bash create-parent-chart-values.sh

Signed-off-by: Mingmin Xu <[email protected]>
Signed-off-by: Mingmin Xu <[email protected]>
add dd_profile in CN pods

Signed-off-by: Mingmin Xu <[email protected]>
@mingmxu mingmxu force-pushed the issue-423/datadog-helm-conf branch from cc1e56d to e2f66b1 Compare February 2, 2024 18:00
@@ -282,6 +287,13 @@ starrocksCnSpec:
# If runAsNonRoot is true, the container is run as non-root user.
# The userId will be set to 1000, and the groupID will be set to 1000.
runAsNonRoot: false
# add/drop capabilities for CN container.
capabilities: { }
Copy link
Collaborator

Choose a reason for hiding this comment

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

an space in { }

@yandongxiao yandongxiao merged commit 5853f40 into StarRocks:main Feb 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants