Update Go modules #1270
Update Go modules #1270
Conversation
SuperQ
commented
May 5, 2023
SuperQ
commented
- Update Go modules for new protobuf version.
- Update Go modules in sub projects.
- Enable dependabot for sub projects.
* Update Go modules for new protobuf version. * Update Go modules in sub projects. * Enable dependabot for sub projects. * Update golangci-lint action. * Update CircleCI orb. Signed-off-by: SuperQ <[email protected]>
SuperQ
force-pushed
the
superq/client_model
branch
from
1b36c0d to
d7d7c75
Compare
|
This is going to require a bunch of test refactoring. |
|
Please ping me again when the tests are passing. :) |
Signed-off-by: SuperQ <[email protected]>
|
General idea looks OK. But there are even more code locations where we have utilized the text formatted protobuf, which still fail. I guess you don't even need to change from angle bracket to curly braces with the new comparison helper. Also, relevant lint warnings and now a merge conflict (which, I'm afraid, has to do with even more tests). |
|
Yea, I'm not sure about what to do to fix a few of these, which is why I did only a partial update so far.
Ideas:
|
There was a problem hiding this comment.
Thanks for this! Generally looking good.
Indeed we could write small function to convert new output to old, but we might as well refactor as you do now in one go (thanks for amazing work!). Please make sure to keep Examples in, they are important to have example type (for documentation reasons).
Tests for scrape output that test text proto.
Inline text proto errors in the text scrape format.
Can you point us to problematic parts? Would love to take closer look on what's best approach there 🤗
| @@ -106,7 +106,7 @@ func (v *InfoVec) MustCurryWith(labels prometheus.Labels) *InfoVec { | |||
| return vec | |||
| } | |||
|
|
|||
| func ExampleMetricVec() { | |||
| func TestExampleMetricVec(t *testing.T) { | |||
There was a problem hiding this comment.
I think it meant to be Example for a reason (it's nicely highlighted on Go docs) 🤔
|
@bwplotka When we print some errors in the registry output, we embed part of the protobuf as output. For example this section of registry.go. We check for errors like: |
|
Ok, but that is again - expecting in unit test a different format, so we just need to switch to new text format in expected output? 🤔 |
|
@bwplotka No, it's not just that. The new protobuf |
|
Let me try calling |
|
For the Go example use case, the non-deterministic text output is really annoying. We cannot even implement our own comparison function (like one that ignores whitespace). |
|
So there is some truth that tests comparing text encoding is a little brittle and blocks serialization innovations (?). If we follow suggestions I think we have to get rid of checking exact errors and parse text format or proto format to structure and compare semantically those (e.g. here) |
|
But examples, yea.. but it's also not text encoding, it's our string that just happen to use this to show what metrics we talk about also on errors. Wonder how hard would be to write our our strangifier or unrandomizing it by |
|
Will try to help here |
|
Closing in favor of #1323 |
