update container names to match the ones expected by cortex-mixin

[danfromtitan]

What this PR does:
All Cortex container names take the Chart.Name value, making it impossible to query metrics based on container metric label. On top of that, the cortex-mixin configuration looks for specific names. The change aligns the container names with their functionality.

Which issue(s) this PR fixes:
Fixes #233

Checklist

• CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

[nschad]

LGTM

[nschad]

Hey @danfromtitan,

I use the mixin as well. I can share my config in order to get it working. I use some relabel config to add the cluster part/label and slighty adjusted job labels. I did not have to rename containers, that really doesn't make super much sense too me either

Soo..

distributor:
  service:
    labels:
      cluster: my-cluster
  serviceMonitor:
    enabled: true
    relabelings:
      - sourceLabels: [__meta_kubernetes_service_label_cluster]
        regex: (.*)
        replacement: $1
        targetLabel: cluster
        action: replace
      - sourceLabels: [__meta_kubernetes_namespace, __meta_kubernetes_service_label_app_kubernetes_io_component]
        targetLabel: job
        separator: "/"

And as job_names I have just

     job_names: {
      ingester: 'ingester',  // Match also ingester-blocks, which is used during the migration from chunks to blocks.
      distributor: 'distributor',

[danfromtitan]

I can share my config in order to get it working

Thanks for sharing, I was looking for this relabeling detail earlier, I think it would reduce the amount of changes required in cortex-mixin fork but it would still need container to have the component in the name for the reasons mentioned above.

[nschad]

I can share my config in order to get it working

Thanks for sharing, I was looking for this relabeling detail earlier, I think it would reduce the amount of changes required in cortex-mixin fork but it would still need container to have the component in the name for the reasons mentioned above.

Yep I agree. I also didn't check every dashboard, so there still might be some problems. I feel like this is a bit out of scope but maybe we can figure something out. Thanks for raising the Issue

[kd7lxl]

Have you tested the relabeling with/without the container name change? It's still unclear whether this change is proven useful or just something you wanted to test.

[danfromtitan]

I did, the relabelling helps whenever a namespace/job value comes into play, but container labels are hard coded as I mentioned above.

Besides the relabelling and the container names change in the chart, you can see here the additional changes required to get all dashboards fully functional.

[nschad]

I did, the relabelling helps whenever a namespace/job value comes into play, but container labels are hard coded as I mentioned above.

Besides the relabelling and the container names change in the chart, you can see here the additional changes required to get all dashboards fully functional.

Have you created a PR over at cortex-jsonnet to be more dynamic with the mixin?

[danfromtitan]

Have you created a PR over at cortex-jsonnet to be more dynamic with the mixin?

That was my next target, but I cannot test the changes there unless this PR is merged first.

[nschad]

Have you created a PR over at cortex-jsonnet to be more dynamic with the mixin?

That was my next target, but I cannot test the changes there unless this PR is merged first.

Excuse my ignorance but why? Which dashboards require a change here? I don't really get it

[danfromtitan]

Excuse my ignorance but why? Which dashboards require a change here? I don't really get it

Please see in my comment above.

[nschad]

Excuse my ignorance but why? Which dashboards require a change here? I don't really get it

Please see in my comment above.

Gotcha! I will talk to @kd7lxl

[kd7lxl]

I'm a little puzzled by the conversation here.

I cannot test the changes there unless this PR is merged first.

This doesn't make any sense to me. You can helm install from your branch just as easily as you can helm install from the master branch or a release.

Please test the desired dashboard behavior against this branch to prove that it solves the problem. Please share your results.

If everything works as expected, we can target this for v1.0.0. It will need to be in a major version release because it's a breaking change -- everyone who has dashboards targeting the current container names will need to rewrite their dashboards.

[danfromtitan]

everyone who has dashboards targeting the current container names will need to rewrite their dashboards

In case someone's using cortex-mixin to create dashboards, they will need the same change I requested in this PR.

Anyone else using container label in their dashboards would look at misleading results, because container label "cortex" would query the metrics from all components instead of the specific component dashboard is intended for.

I have made changes to the cortex-mixin project that are waiting to be merged, dahboards work perfectly, we need this PR merged first before I can submit the other.

[nschad]

everyone who has dashboards targeting the current container names will need to rewrite their dashboards

In case someone's using cortex-mixin to create dashboards, they will need the same change I requested in this PR.

Anyone else using container label in their dashboards would look at misleading results, because container label "cortex" would query the metrics from all components instead of the specific component dashboard is intended for.

I have made changes to the cortex-mixin project that are waiting to be merged, dahboards work perfectly, we need this PR merged first before I can submit the other.

That's fine by me. We can release that with version 1.0.0

[nschad]

@danfromtitan have you opened up a PR with the folks over at grafana?

[danfromtitan]

Not yet, it's on my list. Thanks for merging this.

[danfromtitan]

@danfromtitan have you opened up a PR with the folks over at grafana?

Thanks for accepting this PR, I opened grafana/cortex-jsonnet#427 to fix the outstanding issues with Grafana dashboards when Cortex is deployed from the Helm chart.