Skip to content
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

Add Windows service trigger_start filter to config #15975

Closed
wants to merge 54 commits into from

Conversation

mrafi97
Copy link
Contributor

@mrafi97 mrafi97 commented Oct 6, 2023

What does this PR do?

Adds a new trigger_start pattern to the windows_service services config entry that enables users to filter services by whether or not they have triggers.

This new pattern combines with the existing patterns:

# Matches all services that do not have a trigger
services:
  - trigger_start: false

# Matches all services with an automatic startup type and excludes services with triggers
services:
  - startup_type: automatic
    trigger_start: false

# Only matches EventLog service when its startup type is automatic and has triggers
services:
  - name: EventLog
    startup_type: automatic
    trigger_start: true

Motivation

Jira: https://datadoghq.atlassian.net/browse/WINA-105

builds upon #12940 to allow service filtering based on name, startup_type and trigger_start

Additional Notes

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Changelog entries must be created for modifications to shipped code
  • Add the qa/skip-qa label if the PR doesn't need to be tested during QA.

@github-actions
Copy link

github-actions bot commented Oct 6, 2023

The validations job has failed; please review the Files changed tab for possible suggestions to resolve.

@mrafi97 mrafi97 requested a review from a team as a code owner October 6, 2023 22:02
@ghost ghost added the documentation label Oct 6, 2023
@codecov
Copy link

codecov bot commented Oct 6, 2023

Codecov Report

Merging #15975 (ba41040) into master (2e3ce8a) will increase coverage by 0.20%.
Report is 44 commits behind head on master.
The diff coverage is 98.16%.

Flag Coverage Δ
active_directory 100.00% <ø> (+17.64%) ⬆️
activemq 52.87% <ø> (?)
activemq_xml 82.31% <ø> (ø)
amazon_msk 89.07% <ø> (ø)
ambari 85.75% <ø> (ø)
apache 95.08% <ø> (ø)
argocd 87.81% <ø> (ø)
aspdotnet 100.00% <ø> (ø)
avi_vantage 91.35% <ø> (ø)
boundary 100.00% <ø> (ø)
cacti 87.90% <ø> (ø)
cassandra_nodetool 93.16% <ø> (ø)
cert_manager 77.41% <ø> (ø)
cilium 77.63% <ø> (+0.87%) ⬆️
cisco_aci 95.27% <ø> (ø)
citrix_hypervisor 87.50% <ø> (ø)
cloud_foundry_api 96.35% <ø> (+0.12%) ⬆️
cloudera 99.49% <ø> (ø)
cockroachdb 91.52% <ø> (ø)
confluent_platform 81.96% <ø> (?)
consul 91.65% <ø> (ø)
coredns 94.57% <ø> (ø)
couch 95.43% <ø> (+0.24%) ⬆️
couchbase 84.28% <ø> (ø)
crio 89.79% <ø> (ø)
datadog_checks_base 89.18% <ø> (-0.13%) ⬇️
datadog_checks_dev 82.49% <100.00%> (+0.07%) ⬆️
datadog_checks_downloader 81.65% <ø> (ø)
datadog_cluster_agent 90.19% <ø> (ø)
ddev 99.48% <99.71%> (+0.09%) ⬆️
directory 96.09% <ø> (+0.65%) ⬆️
disk 91.36% <ø> (+2.13%) ⬆️
dns_check 93.90% <ø> (ø)
dotnetclr 91.39% <ø> (+12.90%) ⬆️
druid 98.47% <ø> (ø)
ecs_fargate 82.91% <ø> (ø)
envoy 95.09% <ø> (+0.41%) ⬆️
etcd 95.56% <ø> (ø)
exchange_server 96.85% <ø> (+11.81%) ⬆️
external_dns 89.28% <ø> (ø)
fluentd 94.77% <ø> (ø)
foundationdb 78.50% <ø> (ø)
gearmand 78.26% <ø> (+1.24%) ⬆️
gitlab_runner 91.94% <ø> (ø)
go_expvar 92.73% <ø> (ø)
gunicorn 92.85% <ø> (ø)
haproxy 95.13% <ø> (+0.16%) ⬆️
harbor 89.68% <ø> (ø)
hazelcast 92.39% <ø> (ø)
hdfs_datanode 89.74% <ø> (ø)
hdfs_namenode 86.72% <ø> (ø)
hive ?
hivemq ?
http_check 95.51% <ø> (+2.05%) ⬆️
hudi 73.91% <ø> (ø)
ibm_ace 91.89% <ø> (ø)
ibm_db2 95.35% <ø> (ø)
ibm_mq 91.40% <ø> (+0.13%) ⬆️
ibm_was 96.08% <ø> (ø)
ignite 46.66% <ø> (ø)
iis 95.05% <ø> (ø)
impala 97.97% <ø> (ø)
istio 77.74% <ø> (+0.52%) ⬆️
kafka 63.63% <ø> (ø)
kong 87.56% <ø> (ø)
kube_apiserver_metrics 98.14% <ø> (ø)
kube_controller_manager 96.07% <ø> (ø)
kube_dns 95.97% <ø> (ø)
kube_metrics_server 94.87% <ø> (ø)
kube_proxy 96.80% <ø> (ø)
kube_scheduler 97.63% <100.00%> (+0.13%) ⬆️
kubelet 90.97% <ø> (ø)
kubernetes_state 89.18% <ø> (ø)
lighttpd 83.64% <ø> (ø)
linkerd 85.14% <ø> (+1.14%) ⬆️
linux_proc_extras 96.22% <ø> (ø)
mapr 82.70% <ø> (ø)
marathon 83.43% <ø> (ø)
mcache 93.50% <ø> (ø)
mesos_master 89.75% <ø> (ø)
mesos_slave 93.63% <ø> (ø)
nagios 89.01% <ø> (ø)
network 93.64% <ø> (+1.62%) ⬆️
nfsstat 95.20% <ø> (ø)
nginx 95.24% <ø> (+0.54%) ⬆️
nginx_ingress_controller 98.36% <ø> (ø)
openldap 96.79% <ø> (ø)
openmetrics 98.08% <ø> (ø)
openstack 51.45% <ø> (ø)
oracle 88.63% <ø> (ø)
pdh_check 97.82% <ø> (ø)
pgbouncer 91.33% <ø> (ø)
php_fpm 90.53% <ø> (+1.12%) ⬆️
postfix 88.04% <ø> (ø)
powerdns_recursor 96.65% <ø> (ø)
presto 59.09% <ø> (ø)
process 85.42% <ø> (+0.28%) ⬆️
prometheus 94.17% <ø> (ø)
proxysql 98.97% <ø> (ø)
pulsar 100.00% <ø> (ø)
ray 96.45% <ø> (ø)
redisdb 87.78% <ø> (ø)
rethinkdb 97.93% <ø> (ø)
riak 99.22% <ø> (ø)
riakcs 93.61% <ø> (ø)
scylla 99.49% <ø> (ø)
silk 93.82% <ø> (ø)
singlestore 90.81% <ø> (ø)
solr 56.25% <ø> (ø)
spark 93.91% <ø> (ø)
squid 100.00% <ø> (ø)
statsd 87.36% <ø> (+1.05%) ⬆️
strimzi 89.70% <ø> (ø)
supervisord 90.14% <ø> (ø)
system_core 92.66% <ø> (ø)
system_swap 98.30% <ø> (ø)
tcp_check 92.92% <ø> (ø)
teamcity 88.74% <ø> (+3.21%) ⬆️
temporal 100.00% <ø> (ø)
teradata 94.06% <ø> (ø)
tls 92.18% <ø> (+0.82%) ⬆️
tokumx 58.40% <ø> (ø)
tomcat ?
torchserve 97.33% <ø> (ø)
traffic_server 96.13% <ø> (ø)
twemproxy 79.45% <ø> (ø)
twistlock 79.62% <ø> (ø)
varnish 84.39% <ø> (+0.26%) ⬆️
vault 95.53% <ø> (+0.57%) ⬆️
voltdb 96.85% <ø> (ø)
weblogic 71.73% <ø> (ø)
win32_event_log 86.98% <ø> (+0.66%) ⬆️
windows_performance_counters 98.36% <ø> (ø)
windows_service 94.75% <80.00%> (-3.26%) ⬇️
wmi_check 92.91% <ø> (ø)
yarn 89.59% <ø> (ø)
zk 82.18% <ø> (+0.89%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@github-actions
Copy link

github-actions bot commented Oct 6, 2023

Test Results

  4 files    4 suites   38s ⏱️
20 tests 18 ✔️ 2 💤 0
42 runs  36 ✔️ 6 💤 0

Results for commit c16939d.

♻️ This comment has been updated with latest results.

windows_service/README.md Outdated Show resolved Hide resolved
aliciascott
aliciascott previously approved these changes Oct 10, 2023
Copy link
Contributor

@aliciascott aliciascott left a comment

Choose a reason for hiding this comment

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

Hi @mrafi97 just a small change, otherwise approved!

windows_service/README.md Outdated Show resolved Hide resolved
iglendd
iglendd previously approved these changes Oct 10, 2023
Comment on lines 42 to 43
- startup_type: automatic
trigger_start: false
Copy link
Contributor

@steveny91 steveny91 Oct 10, 2023

Choose a reason for hiding this comment

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

startup_type and trigger_start

Are these two params used exclusively together? Or can either of them be a standalone param?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @steveny91, startup_type and trigger_start can be used standalone they don't need to be used together.

Copy link
Contributor

Choose a reason for hiding this comment

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

Then shouldn't the config example have:

Suggested change
- startup_type: automatic
trigger_start: false
- startup_type: automatic
- trigger_start: false

Since they're standalone?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @steveny91 , Yeah that makes sense. I've added this in my latest commit.

@mrafi97 mrafi97 dismissed stale reviews from iglendd and aliciascott via 7569a1e October 10, 2023 22:29
@mrafi97 mrafi97 force-pushed the mohammad.rafi/windows-service-trigger branch from 7569a1e to afcb544 Compare October 10, 2023 22:33
@iglendd
Copy link
Contributor

iglendd commented Oct 17, 2023

@mrafi97 I think something wrong with the PR now. It shows 247 files changed and all approvals are gone. ... And "branch is out-of date with the base branch"

@mrafi97
Copy link
Contributor Author

mrafi97 commented Oct 17, 2023

@iglendd I might have to close this one and open a new one, could be that my branch is much farther behind in commits than expected.

@mrafi97 mrafi97 closed this Oct 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment