[cmd/mdatagen] Add lint/ordering validation for metadata.yaml#13782
Conversation
Signed-off-by: Paulo Dias <paulodias.gm@gmail.com>
Signed-off-by: Paulo Dias <paulodias.gm@gmail.com>
Signed-off-by: Paulo Dias <paulodias.gm@gmail.com>
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (83.01%) is below the target coverage (95.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #13782 +/- ##
==========================================
- Coverage 91.61% 91.59% -0.02%
==========================================
Files 655 655
Lines 42729 42782 +53
==========================================
+ Hits 39147 39188 +41
- Misses 2761 2769 +8
- Partials 821 825 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Paulo Dias <paulodias.gm@gmail.com>
Signed-off-by: Paulo Dias <paulodias.gm@gmail.com>
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
/label -stale never-stale |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
The proposal LGTM. @paulojmdias are you planning to finalize the PR? |
|
@dmitryax yes, I will work into it soon |
… into feat/13781 Signed-off-by: Paulo Dias <paulodias.gm@gmail.com>
Signed-off-by: Paulo Dias <paulodias.gm@gmail.com>
Signed-off-by: Paulo Dias <paulodias.gm@gmail.com>
Signed-off-by: Paulo Dias <paulodias.gm@gmail.com>
|
@dmitryax I think you can review now. All the reordering of The The |
dmitryax
left a comment
There was a problem hiding this comment.
Do you have any script or temporary command to update existing files? It'll be painful to sort all the metadata.yaml files in contrib manually
#### Description Sort metadata.yaml file in preparation for sort validation using mdatagen. Context: open-telemetry/opentelemetry-collector#13782 Signed-off-by: Paulo Dias <paulodias.gm@gmail.com>
…3465) #### Description Sort metadata.yaml file in preparation for sort validation using mdatagen. Context: open-telemetry/opentelemetry-collector#13782 Signed-off-by: Paulo Dias <paulodias.gm@gmail.com>
#### Description Sort metadata.yaml file in preparation for sort validation using mdatagen. Context: open-telemetry/opentelemetry-collector#13782 Signed-off-by: Paulo Dias <paulodias.gm@gmail.com>
…43450) #### Description Sort metadata.yaml file in preparation for sort validation using mdatagen. Context: open-telemetry/opentelemetry-collector#13782 Signed-off-by: Paulo Dias <paulodias.gm@gmail.com>
#43454) #### Description Sort metadata.yaml file in preparation for sort validation using mdatagen. Context: open-telemetry/opentelemetry-collector#13782 Signed-off-by: Paulo Dias <paulodias.gm@gmail.com>
…tries (#43446) #### Description Sort metadata.yaml file in preparation for sort validation using mdatagen. Context: open-telemetry/opentelemetry-collector#13782 Signed-off-by: Paulo Dias <paulodias.gm@gmail.com>
…43445) #### Description Sort metadata.yaml file in preparation for sort validation using mdatagen. Context: open-telemetry/opentelemetry-collector#13782 Signed-off-by: Paulo Dias <paulodias.gm@gmail.com>
…43441) #### Description Sort metadata.yaml file in preparation for sort validation using mdatagen. Context: open-telemetry/opentelemetry-collector#13782 Signed-off-by: Paulo Dias <paulodias.gm@gmail.com>
|
@dmitryax, as all the sort PRs on the contrib side were already merged, and the tests are ok now, this PR could be safely reviewed 🙏 |
mx-psi
left a comment
There was a problem hiding this comment.
Ideally we would instead turn this section into a list, since map keys do not have an order in YAML which in practice means that YAML tooling may reorder these as they please
Given that this is underway we can finish with the current batch of PRs on contrib and just file an issue about this and handle it later?
To be on the same page, the idea is to merge all the contrib PRs and close this one, or merge this one and open a new issue to transform everything into a list later on? |
We can merge all contrib PRs, merge this one and file an issue to deal with what I mentioned |
|
@mx-psi LGTM 👍 |
) #### Description Sort metadata.yaml file in preparation for sort validation using mdatagen. Context: open-telemetry/opentelemetry-collector#13782 Signed-off-by: Paulo Dias <paulodias.gm@gmail.com>
#### Description Sort metadata.yaml file in preparation for sort validation using mdatagen. Context: open-telemetry/opentelemetry-collector#13782 Signed-off-by: Paulo Dias <paulodias.gm@gmail.com>
#### Description Sort metadata.yaml file in preparation for sort validation using mdatagen. Context: open-telemetry/opentelemetry-collector#13782 Signed-off-by: Paulo Dias <paulodias.gm@gmail.com>
dmitryax
left a comment
There was a problem hiding this comment.
@paulojmdias please rebase. It should fix the CI I hope
It's fine. Coverage of the mdatagen command isn't that important as the collector code |
Description
Extend mdatagen to validate that keys in metadata.yaml files are alphabetically sorted (resource_attributes, attributes, metrics, events, and telemetry.metrics).
The idea came from open-telemetry/opentelemetry-collector-contrib#42477 (comment)
Link to tracking issue
Fixes #13781
Testing
I added tests to validate unordered data and expect an error on it.