Add YAMLLint to CI

[mowies]

Fixes #1688

Changes

• added yamllint CLI to the CI pipeline to check all YAML files in the repo
• adapted contributing guidelines to reflect changes as well
• added default linter ruleset copied from https://github.com/open-telemetry/opentelemetry-demo/blob/main/.yamllint

Note

There's are lots of errors showing right now, using the default ruleset.
For now, I copied the ruleset from https://github.com/open-telemetry/opentelemetry-demo/blob/main/.yamllint which seems reasonable to me, but there's still some linting errors. I can definitely fix the remaining linter errors if somebody here tells me to :)

Merge requirement checklist

• CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
• Unit tests added/updated
• Appropriate CHANGELOG.md files updated for non-trivial changes
• Changes in public API reviewed (if applicable)

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: mowies / name: Moritz Wiesinger (bdaa88b, f3c410b, 9f414c4, b2589fa, a4bea41, dc21777, 1c57e66, 9bc8e3c, 5c35aae, 648442c)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.70%. Comparing base (71655ce) to head (648442c).
Report is 350 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1933      +/-   ##
==========================================
- Coverage   73.91%   70.70%   -3.21%     
==========================================
  Files         267      298      +31     
  Lines        9615    11133    +1518     
==========================================
+ Hits         7107     7872     +765     
- Misses       2508     3261     +753     

Flag	Coverage Δ
unittests-Exporter.Geneva	53.20% <ø> (?)
unittests-Exporter.InfluxDB	95.88% <ø> (?)
unittests-Exporter.Instana	71.24% <ø> (?)
unittests-Exporter.OneCollector	91.29% <ø> (?)
unittests-Exporter.Stackdriver	75.73% <ø> (?)
unittests-Extensions	79.33% <ø> (?)
unittests-Extensions.AWS	77.24% <ø> (?)
unittests-Extensions.Enrichment	100.00% <ø> (?)
unittests-Instrumentation.AWS	87.56% <ø> (?)
unittests-Instrumentation.AWSLambda	87.96% <ø> (?)
unittests-Instrumentation.AspNetCore	85.27% <ø> (?)
unittests-Instrumentation.ElasticsearchClient	79.87% <ø> (?)
unittests-Instrumentation.EntityFrameworkCore	55.49% <ø> (?)
unittests-Instrumentation.EventCounters	76.36% <ø> (?)
unittests-Instrumentation.GrpcNetClient	79.61% <ø> (?)
unittests-Instrumentation.Hangfire	93.58% <ø> (?)
unittests-Instrumentation.Http	81.08% <ø> (?)
unittests-Instrumentation.Process	100.00% <ø> (?)
unittests-Instrumentation.Quartz	78.94% <ø> (?)
unittests-Instrumentation.Runtime	100.00% <ø> (?)
unittests-Instrumentation.SqlClient	90.90% <ø> (?)
unittests-Instrumentation.StackExchangeRedis	67.02% <ø> (?)
unittests-Instrumentation.Wcf	48.91% <ø> (?)
unittests-PersistentStorage	65.44% <ø> (?)
unittests-Resources.AWS	77.93% <ø> (?)
unittests-Resources.Azure	77.94% <ø> (?)
unittests-Resources.Container	72.41% <ø> (?)
unittests-Resources.Gcp	72.54% <ø> (?)
unittests-Resources.Host	47.67% <ø> (?)
unittests-Resources.Process	81.81% <ø> (?)
unittests-Resources.ProcessRuntime	72.91% <ø> (?)
unittests-Sampler.AWS	87.97% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 323 files with indirect coverage changes

[Kielek]

@mowies, thanks for doing this.

As you described, this job is failing: https://github.com/open-telemetry/opentelemetry-dotnet-contrib/actions/runs/9739790217/job/26875671492?pr=1933.
Before merging we need to fix it.
I have a couple of comments/questions there

1. Lister reports both Errors and Warnings. What happen if it will have only warnings? Will it fail or just left a comment? If it only left a comment in the execution, we should either disabled it, or better treat as errors. We do not need to soft recommendation. Nobody will be checking the CI output.
2. Line length - I think that we could just disabled this/double the value to 400. I do not see this as problematic (just personal opinion, you can fix all occurrences to 200).
3. Please fix all other, obvious issues reporter by the linter.

[mowies]

1. Lister reports both Errors and Warnings. What happen if it will have only warnings? Will it fail or just left a comment? If it only left a comment in the execution, we should either disabled it, or better treat as errors. We do not need to soft recommendation. Nobody will be checking the CI output.

So we have 2 options for this: strict mode and ignoring warnings

• strict mode: yamllint will treat warnings as errors and will return with a non-zero exit code also when there's just warnings
• ignoring warnings: yamllint will not print any warnings, just errors

2. Line length - I think that we could just disabled this/double the value to 400. I do not see this as problematic (just personal opinion, you can fix all occurrences to 200).

I will disable the rule.

[mowies]

@Kielek fixed all current linter errors, ignored the warnings for now. Please see my previous comment :)

[Kielek]

@movies, I think that we can ignore all warnings in this PR and enable strict mode in follow up/disable unwanted warnings one by one.

Now it pulutes output and PRs:

BTW, it will be great if you allow maintainers to modify your pr-branch. With this, we can merge all changes from current main, and merge it without waiting for you.

[mowies]

@movies, I think that we can ignore all warnings in this PR and enable strict mode in follow up/disable unwanted warnings one by one.

Sounds good @Kielek , I changed the command to include --no-warnings and updated the contrib guide accordingly.

BTW, it will be great if you allow maintainers to modify your pr-branch. With this, we can merge all changes from current main, and merge it without waiting for you.

It seems that I can't, since my fork is owned by an organization and not my personal account...
But I merged main now.

[Kielek]

@mowies, could you please merge main branch to your PR branch? I think that we are good to merge.

Consider creating follow up issues/PRs with enabling more strict validation.