feat: add multiple features and fix #39

coanor · 2023-08-15T18:13:11Z

Enable list metrics in vendor packages(--with-vendor)
- Why? Some vendor packages also export metrics, we need to export them for full metric documents.
List labels of metrics, if no labels, output N/A(Add label-name list in exported output #37)
Fix panic on --add-help if metric got no help info, and also if no help output N/A
Out JSON with pretty indents
Output markdown table(Need markdown table export #38)
Avoid duplicated metric name(Duplicated metric name on chan expr #40)

- Enable list metrics in vendor packages(--with-vendor) - List labels of metrics(if no labels, output `N/A') - Fix panic on --add-help if metric got no help info

coanor · 2023-08-15T18:24:06Z

The output example seems like this:

TYPE                NAME                    LABELS       HELP
COUNTER             http_api_signed_total   api,method   API signature count
COUNTER             http_api_dropped_total  api,method   API request dropped when sinker rule match failed

Labels are , splited string. If there're const labels, the label displayed as:

some_const_label=some_value: The const label got a string literal value
some_const_label=?: While the const label value are not string literal(such as from fmt.Sprintf() or someModule.SomeValue)

And the JSON format like this:

[
  {
    "Name": "http_api_dropped_total",
    "Help": "API request dropped when sinker rule match failed",
    "Type": "COUNTER",
    "Filename": "/path/to/metrics.go",
    "Labels": [
      "api",
      "method"
    ],
    "Line": 105,
    "Column": 3
  },
  {
    "Name": "http_api_signed_total",
    "Help": "API signature count",
    "Type": "COUNTER",
    "Filename": "/path/to/metrics.go",
    "Labels": [
      "api",
      "method"
    ],
    "Line": 118,
    "Column": 3
  }
]

The markdown table seems like this:

| TYPE    | NAME                     | LABELS       | HELP                                              |
| ---     | ---                      | ---          | ---                                               |
| COUNTER | `http_api_signed_total`  | `api,method` | API signature count                               |
| COUNTER | `http_api_dropped_total` | `api,method` | API request dropped when sinker rule match failed |

yeya24 · 2023-08-16T18:01:02Z

Thanks for the good work! @coanor Are you able to add some unit tests about the labels feature?

- Fix duplicated metric name output

coanor · 2023-08-20T03:03:33Z

Thanks for the good work! @coanor Are you able to add some unit tests about the labels feature?

I find the PR can't find labels within NewDesc, I need some time to refactor current parseXXX functions. And current PR closed, I'll reopen it after variableLabels and constLabels able to parse.

coanor · 2023-08-20T08:46:55Z

Thanks for the good work! @coanor Are you able to add some unit tests about the labels feature?

I've add label discovery within NewDesc and basic test case, but it seems that the original case within promlinter_test.go failed, there missing a issue in current MR, currently I have no idea about what happend.

Will you give me some hint about why the case failed?

Print position(file name or path) in italic under markdown table output

yeya24 · 2023-09-18T05:36:05Z

It seems like the error case below was missing.

	if issues[2].Metric != "test_metric_total" && issues[2].Text != `no help text` {
		t.Fatal()
	}

coanor · 2023-09-24T06:26:44Z

It seems like the error case case below was missing.
	if issues[2].Metric != "test_metric_total" && issues[2].Text != `no help text` {
		t.Fatal()
	}

I've refactor and fixed the testings within promlinter_test.go, It seems that the issue's order not the key concerning.

yeya24 · 2023-10-01T08:33:47Z

promlinter.go

+	}
+
+	if !opts.helpSet {
+		currentMetric.Help = nil


We don't need this condition. We cannot set String to nil I guess.
Line 327 should be enough to check if help is set

currentMetric.Help is type *string, we just assign nil to mean "There is no help available". If we set a point to empty string("", here is line 327's opts.help, it's default value is ""), some exists test cases will fail(empty string still a valid help, but not make sense.).

promlinter.go

yeya24 · 2023-10-01T08:36:52Z

I've refactor and fixed the testings within promlinter_test.go, It seems that the issue's order not the key concerning.

I am ok to refactor the tests but I think it is still good to return ordered (at least order can be determined) issues from RunLint().

coanor · 2023-10-12T16:46:38Z

I've refactor and fixed the testings within promlinter_test.go, It seems that the issue's order not the key concerning.

I am ok to refactor the tests but I think it is still good to return ordered (at least order can be determined) issues from RunLint().

About the sort, I think we do not need to sort the result issue, because they are sorted already according to the AST traverse(And I removed the trailing sort on issues(fd4631d)).

For example, the raw issue list is(from testdata/testdata.go)

test_metric_name: ./testdata/testdata.go:23:3
test_metric_total: ./testdata/testdata.go:32:3
foo: ./testdata/testdata.go:39:30
foo_bar_total: ./testdata/testdata.go:78:8
kube_test_metric_count: ./testdata/testdata.go:92:8
test_histogram_duration_seconds: ./testdata/testdata.go:95:27
kube_daemonset_labels: ./testdata/testdata.go:119:4

After sort(based on pos.String()), the ording changed(and is not the AST order):

kube_daemonset_labels: ./testdata/testdata.go:119:4
test_metric_name: ./testdata/testdata.go:23:3
test_metric_total: ./testdata/testdata.go:32:3
foo: ./testdata/testdata.go:39:30
foo_bar_total: ./testdata/testdata.go:78:8
kube_test_metric_count: ./testdata/testdata.go:92:8
test_histogram_duration_seconds: ./testdata/testdata.go:95:27

Because string a.go:123 < a.go:99, and that not the order we need：

import (
  T "testing"

  "github.com/stretchr/testify/assert"
)

func TestStringOrder(t *T.T) {
	assert.True(t, "a.go:123" < "a.go:99")  // Pass
}

yeya24 · 2024-04-22T17:46:51Z

Sorry for taking it for so long. I am going to merge it. Great work! Thanks

coanor added 2 commits August 16, 2023 02:07

feat: add multiple features

4546dd3

- Enable list metrics in vendor packages(--with-vendor) - List labels of metrics(if no labels, output `N/A') - Fix panic on --add-help if metric got no help info

fix: fix typo

fd52992

feat: add markdown table output

d8a0f50

- Fix duplicated metric name output

coanor closed this Aug 20, 2023

coanor changed the title ~~feat: add multiple features~~ feat: add multiple features and fix Aug 20, 2023

coanor added 2 commits August 20, 2023 15:44

add label and const label discovery

ad2b480

add test on list labels

36c2c30

coanor reopened this Aug 20, 2023

update test cases

68e2fb7

feat: add print option --add-module

4f66f05

Print position(file name or path) in italic under markdown table output

coanor added 2 commits September 24, 2023 11:53

save

5bfbd83

fix unit test fail

854cc71

remove debugging logging

76c8a1d

yeya24 reviewed Oct 1, 2023

View reviewed changes

coanor added 3 commits October 13, 2023 00:07

remove noise comment.

1ce3763

save

0365be5

remove sort on result issue.

fd4631d

Merge branch 'master' into master

5fdcadf

yeya24 approved these changes Apr 22, 2024

View reviewed changes

yeya24 merged commit 66dbcb5 into yeya24:master Apr 22, 2024
1 check failed

yeya24 mentioned this pull request Apr 22, 2024

Duplicated metric name on chan expr #40

Closed

yeya24 mentioned this pull request Apr 22, 2024

Need markdown table export #38

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add multiple features and fix #39

feat: add multiple features and fix #39

coanor commented Aug 15, 2023 •

edited

Loading

coanor commented Aug 15, 2023 •

edited

Loading

yeya24 commented Aug 16, 2023

coanor commented Aug 20, 2023 •

edited

Loading

coanor commented Aug 20, 2023

yeya24 commented Sep 18, 2023 •

edited

Loading

coanor commented Sep 24, 2023 •

edited

Loading

yeya24 Oct 1, 2023

coanor Oct 12, 2023 •

edited

Loading

yeya24 commented Oct 1, 2023

coanor commented Oct 12, 2023 •

edited

Loading

yeya24 commented Apr 22, 2024 •

edited

Loading

feat: add multiple features and fix #39

feat: add multiple features and fix #39

Conversation

coanor commented Aug 15, 2023 • edited Loading

coanor commented Aug 15, 2023 • edited Loading

yeya24 commented Aug 16, 2023

coanor commented Aug 20, 2023 • edited Loading

coanor commented Aug 20, 2023

yeya24 commented Sep 18, 2023 • edited Loading

coanor commented Sep 24, 2023 • edited Loading

yeya24 Oct 1, 2023

Choose a reason for hiding this comment

coanor Oct 12, 2023 • edited Loading

Choose a reason for hiding this comment

yeya24 commented Oct 1, 2023

coanor commented Oct 12, 2023 • edited Loading

yeya24 commented Apr 22, 2024 • edited Loading

coanor commented Aug 15, 2023 •

edited

Loading

coanor commented Aug 15, 2023 •

edited

Loading

coanor commented Aug 20, 2023 •

edited

Loading

yeya24 commented Sep 18, 2023 •

edited

Loading

coanor commented Sep 24, 2023 •

edited

Loading

coanor Oct 12, 2023 •

edited

Loading

coanor commented Oct 12, 2023 •

edited

Loading

yeya24 commented Apr 22, 2024 •

edited

Loading