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.

Sign up for GitHub

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

Jump to bottom

Revert "Loki Prometheus mixin: templatize cluster label" #6377

Merged
merged 2 commits into from
Jun 13, 2022
Merged

Revert "Loki Prometheus mixin: templatize cluster label" #6377

merged 2 commits into from
Jun 13, 2022

Conversation

kavirajk
Copy link
Contributor

Reverts #6353

@hervenicol Reverting the PR, because it breaks the main branch. (it passed on your original branch, but suspect that CI may be bit flaky. But when tried in local make lint-jsonnet it fails as well)

Linting ./production/promtail-mixin/mixin.libsonnet
RUNTIME ERROR: Field does not exist: per_cluster_label
	production/promtail-mixin/vendor/loki-mixin/dashboards/dashboard-utils.libsonnet:34:58-85	thunk from <function <anonymous>>
	production/promtail-mixin/vendor/grafana-builder/grafana.libsonnet:31:57-67	thunk from <object <anonymous>>
	<std>:715:15-22	thunk <val> from <function <format_codes_arr>>
	<std>:722:27-30	thunk from <thunk <s> from <function <format_codes_arr>>>
	<std>:592:22-25	thunk from <function <format_code>>
	<std>:592:9-26	function <format_code>
	<std>:722:15-60	thunk <s> from <function <format_codes_arr>>
	<std>:727:24-25	thunk from <thunk <s_padded> from <function <format_codes_arr>>>
	<std>:480:30-33	thunk from <thunk from <function <pad_left>>>
	<std>:480:19-34	thunk from <function <pad_left>>
	...
	<std>:733:11-64	function <format_codes_arr>
	<std>:777:7-46	function <anonymous>
	<std>:237:7-23	function <anonymous>
	production/promtail-mixin/vendor/grafana-builder/grafana.libsonnet:31:18-68	
	Field "query"	
	Array element 1	
	Field "list"	
	Field "templating"	
	Field "promtail.json"	
	During manifestation

to me it looks like it's failing to lint promtail-mixin because promtail-mixin unable to find this new label per_cluster_label when used with called with addCluster() where multi=false by default.

Will be happy to merge it again, once the CI linter is fixed. :) may be in separate PR.

@kavirajk kavirajk requested a review from a team as a code owner June 13, 2022 09:51
@grafanabot
Copy link
Collaborator

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

dannykopping
dannykopping approved these changes Jun 13, 2022
View reviewed changes
Copy link
Contributor

@dannykopping dannykopping left a comment

Choose a reason for hiding this comment

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

Thanks @kavirajk 👍

@hervenicol
Copy link
Contributor

Oh crap. Sorry 😞
I'll work on it and create a new PR then.

@hervenicol hervenicol mentioned this pull request Jun 13, 2022
Closed
@kavirajk
Replace loki-mixin dependency to working commit.
ac0ea6e 
Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
@grafanabot
Copy link
Collaborator

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

@grafanabot
Copy link
Collaborator

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0.3%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

@kavirajk kavirajk mentioned this pull request Jun 13, 2022
Open
@kavirajk kavirajk merged commit 666bac2 into main Jun 13, 2022
@kavirajk kavirajk deleted the revert-6353-mixins_templatize_clusterlabel branch June 13, 2022 10:57
kavirajk added a commit that referenced this pull request Jun 13, 2022
@kavirajk
chore(jsonnet-lint): Make promtail-mixin depends on loki-mixin `m…
c57359d 
…ain` version

We had to change it to some older version to revert this PR
#6377

Issue, context and long-term fix are explained in this
#6378

Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
@kavirajk kavirajk mentioned this pull request Jun 13, 2022
Merged
kavirajk added a commit that referenced this pull request Jun 13, 2022
@kavirajk
chore(jsonnet-lint): Make promtail-mixin depends on loki-mixin `m…
116d61e 
…ain` version (#6379)

We had to change it to some older version to revert this PR
#6377

Issue, context and long-term fix are explained in this
#6378

Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
JordanRushing pushed a commit to JordanRushing/loki that referenced this pull request Jun 17, 2022
@kavirajk @JordanRushing
Revert "Loki Prometheus mixin: templatize cluster label" (grafana#6377)
38683bd 
* Revert "Loki Prometheus mixin: templatize cluster label (grafana#6353)"

This reverts commit 459036f.

* Replace `loki-mixin` dependency to working commit.

Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
(cherry picked from commit 666bac2)
JordanRushing added a commit that referenced this pull request Jun 17, 2022
@JordanRushing @kavirajk
Backport #6413 and #6377 to k102 (#6422)
372961d 
* Backport memberlist dependency version pin to k102

Signed-off-by: JordanRushing <rushing.jordan@gmail.com>

* Revert "Loki Prometheus mixin: templatize cluster label" (#6377)

* Revert "Loki Prometheus mixin: templatize cluster label (#6353)"

This reverts commit 459036f.

* Replace `loki-mixin` dependency to working commit.

Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
(cherry picked from commit 666bac2)

Co-authored-by: Kaviraj Kanagaraj <kavirajkanagaraj@gmail.com>
grafanabot pushed a commit that referenced this pull request Jun 24, 2022
@JordanRushing @grafanabot
Backport #6413 and #6377 to k102 (#6422)
cd361ce 
* Backport memberlist dependency version pin to k102

Signed-off-by: JordanRushing <rushing.jordan@gmail.com>

* Revert "Loki Prometheus mixin: templatize cluster label" (#6377)

* Revert "Loki Prometheus mixin: templatize cluster label (#6353)"

This reverts commit 459036f.

* Replace `loki-mixin` dependency to working commit.

Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
(cherry picked from commit 666bac2)

Co-authored-by: Kaviraj Kanagaraj <kavirajkanagaraj@gmail.com>
(cherry picked from commit 372961d)
vlad-diachenko pushed a commit that referenced this pull request Jun 24, 2022
@grafanabot @JordanRushing
Backport #6413 and #6377 to k102 (#6422) (#6492)
9e26b87 
* Backport memberlist dependency version pin to k102

Signed-off-by: JordanRushing <rushing.jordan@gmail.com>

* Revert "Loki Prometheus mixin: templatize cluster label" (#6377)

* Revert "Loki Prometheus mixin: templatize cluster label (#6353)"

This reverts commit 459036f.

* Replace `loki-mixin` dependency to working commit.

Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
(cherry picked from commit 666bac2)

Co-authored-by: Kaviraj Kanagaraj <kavirajkanagaraj@gmail.com>
(cherry picked from commit 372961d)

Co-authored-by: JordanRushing <rushing.jordan@gmail.com>
This was referenced Jun 28, 2022
Closed
Draft
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dannykopping dannykopping dannykopping approved these changes

Assignees
No one assigned
Labels
size/M
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kavirajk @grafanabot @hervenicol @dannykopping