acryldata / datahub-helm

Repository of helm charts for deploying DataHub on a Kubernetes cluster
Apache License 2.0
160 stars 239 forks source link

feat: Add custom label support for ServiceMonitor in DataHub charts (#449) #450

Closed Squallman closed 4 months ago

Squallman commented 6 months ago

This pull request adds support for specifying custom labels for ServiceMonitor resources via the values.yaml file in the DataHub Helm charts. This enhancement enables seamless integration with Prometheus by allowing for the application of organization-specific labeling conventions directly during the Helm chart deployment process.

Motivation and Context

Custom label support for ServiceMonitor resources aligns with existing customization capabilities for Service and Deployment resources within the DataHub Helm charts, addressing a gap in the configuration flexibility needed for comprehensive monitoring setups. This update facilitates more precise service discovery and monitoring configurations, particularly in environments where Prometheus's service discovery mechanisms rely heavily on specific label sets.

How Has This Been Tested?

Types of changes

Checklist

Related Issue(s)

Resolves #449

Squallman commented 6 months ago

@david-leifker could you please review?

darnaut commented 5 months ago

@Squallman please rebase and bump the versions for the charts being modified

Squallman commented 5 months ago

@darnaut could you please approve the workflow run?

Masterchen09 commented 5 months ago

@Squallman As we already use your PR within an internal fork of the datahub-helm chart, I noticed, that your changes only affect the service monitors for the datahub-gms and datahub-frontend subcharts, but not the datahub-mae-consumer and datahub-mce-consumer subcharts:

https://github.com/acryldata/datahub-helm/blob/master/charts/datahub/subcharts/datahub-mce-consumer/templates/servicemonitor.yaml

https://github.com/acryldata/datahub-helm/blob/master/charts/datahub/subcharts/datahub-mae-consumer/templates/servicemonitor.yaml

I think it would make sense to also allow to specify custom labels for theses two service monitors. 😊

Squallman commented 4 months ago

Hi @Masterchen09,

I've updated my PR to include the implementation for custom labels in the service monitors for both the datahub-mae-consumer and datahub-mce-consumer subcharts. Could you please review the changes and approve the workflow?

Thank you! 😊

Masterchen09 commented 4 months ago

Hi @Squallman,

thank you for adding the custom labels also for the service monitors of the mae- and mce-consumer! 😊

Unfortunately I cannot review or approve anything here as I am not part of the Acryldata team - I am "only" a contributor like you. πŸ˜