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.

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

[chore] exclude certain gosec linters #33192

Merged

Conversation

codeboten
Copy link
Contributor

This is causing performance issue w/ the linters as discussed in golangci/golangci-lint#4735

This is causing performance issue w/ the linters as discussed in golangci/golangci-lint#4735

Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com>
@codeboten codeboten requested a review from a team as a code owner May 22, 2024 18:06
@codeboten codeboten requested a review from jpkrohling May 22, 2024 18:06
Copy link
Member

@songy23 songy23 left a comment

Choose a reason for hiding this comment

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

Looks like this PR supersedes #33191, maybe you want to close that one?

@songy23 songy23 added the ci-cd CI, CD, testing, build issues label May 22, 2024
@codeboten
Copy link
Contributor Author

Looks like this PR supersedes #33191, maybe you want to close that one?

I'll rebase after that one is merged... want to keep those two changes separate

@codeboten codeboten merged commit b700b34 into open-telemetry:main May 22, 2024
170 checks passed
@codeboten codeboten deleted the codeboten/disable-perf-gosec branch May 22, 2024 18:23
@github-actions github-actions bot added this to the next release milestone May 22, 2024
codeboten added a commit to codeboten/opentelemetry-collector that referenced this pull request May 22, 2024
Same as open-telemetry/opentelemetry-collector-contrib#33192

Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com>
codeboten added a commit to codeboten/opentelemetry-collector that referenced this pull request May 22, 2024
Same as open-telemetry/opentelemetry-collector-contrib#33192

Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com>
codeboten added a commit to open-telemetry/opentelemetry-collector that referenced this pull request May 23, 2024
Same as
open-telemetry/opentelemetry-collector-contrib#33192

---------

Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com>
andrzej-stencel pushed a commit to andrzej-stencel/opentelemetry-collector that referenced this pull request May 27, 2024
…metry#10210)

Same as
open-telemetry/opentelemetry-collector-contrib#33192

---------

Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com>
steves-canva pushed a commit to Canva/opentelemetry-collector that referenced this pull request Jun 14, 2024
…metry#10210)

Same as
open-telemetry/opentelemetry-collector-contrib#33192

---------

Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com>
codeboten pushed a commit that referenced this pull request Jun 14, 2024
**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
The upstream issue was fixed by
golangci/golangci-lint#4748, which was included
in release
[`v1.59.0`](https://github.com/golangci/golangci-lint/releases/tag/v1.59.0)
of `golangci-lint`. From local testing, we're pulling version `v1.59.1`
of `golangci-lint`, so the issue should be resolved.

Local runtime with excludes:
```
$ .tools/golangci-lint run -v --enable-only gosec
...
INFO Execution took 10.927544867s
INFO Execution took 8.011302204s
INFO Execution took 7.716441258s
INFO Execution took 7.441336833s
```
Local runtime without excludes:
```
$ .tools/golangci-lint run -v --enable-only gosec
...
INFO Execution took 9.780250262s
INFO Execution took 8.175492516s
INFO Execution took 7.550060974s
INFO Execution took 7.526585686s
```

Note: I ran `.tools/golangci-lint cache clean` between each test to
clean the cache and keep results as consistent as possible. I admit that
I don't know why the values keep going down with every run, the cache
cleaning command may not entirely be working.

**Link to tracking Issue:** <Issue number if applicable>
These excludes were introduced in
#33192
I've opened a PR in core for this issue as well:
open-telemetry/opentelemetry-collector#10411
t00mas pushed a commit to t00mas/opentelemetry-collector-contrib that referenced this pull request Jun 18, 2024
**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
The upstream issue was fixed by
golangci/golangci-lint#4748, which was included
in release
[`v1.59.0`](https://github.com/golangci/golangci-lint/releases/tag/v1.59.0)
of `golangci-lint`. From local testing, we're pulling version `v1.59.1`
of `golangci-lint`, so the issue should be resolved.

Local runtime with excludes:
```
$ .tools/golangci-lint run -v --enable-only gosec
...
INFO Execution took 10.927544867s
INFO Execution took 8.011302204s
INFO Execution took 7.716441258s
INFO Execution took 7.441336833s
```
Local runtime without excludes:
```
$ .tools/golangci-lint run -v --enable-only gosec
...
INFO Execution took 9.780250262s
INFO Execution took 8.175492516s
INFO Execution took 7.550060974s
INFO Execution took 7.526585686s
```

Note: I ran `.tools/golangci-lint cache clean` between each test to
clean the cache and keep results as consistent as possible. I admit that
I don't know why the values keep going down with every run, the cache
cleaning command may not entirely be working.

**Link to tracking Issue:** <Issue number if applicable>
These excludes were introduced in
open-telemetry#33192
I've opened a PR in core for this issue as well:
open-telemetry/opentelemetry-collector#10411
cparkins pushed a commit to AmadeusITGroup/opentelemetry-collector-contrib that referenced this pull request Jul 11, 2024
This is causing performance issue w/ the linters as discussed in
golangci/golangci-lint#4735

Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com>
cparkins pushed a commit to AmadeusITGroup/opentelemetry-collector-contrib that referenced this pull request Jul 11, 2024
**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
The upstream issue was fixed by
golangci/golangci-lint#4748, which was included
in release
[`v1.59.0`](https://github.com/golangci/golangci-lint/releases/tag/v1.59.0)
of `golangci-lint`. From local testing, we're pulling version `v1.59.1`
of `golangci-lint`, so the issue should be resolved.

Local runtime with excludes:
```
$ .tools/golangci-lint run -v --enable-only gosec
...
INFO Execution took 10.927544867s
INFO Execution took 8.011302204s
INFO Execution took 7.716441258s
INFO Execution took 7.441336833s
```
Local runtime without excludes:
```
$ .tools/golangci-lint run -v --enable-only gosec
...
INFO Execution took 9.780250262s
INFO Execution took 8.175492516s
INFO Execution took 7.550060974s
INFO Execution took 7.526585686s
```

Note: I ran `.tools/golangci-lint cache clean` between each test to
clean the cache and keep results as consistent as possible. I admit that
I don't know why the values keep going down with every run, the cache
cleaning command may not entirely be working.

**Link to tracking Issue:** <Issue number if applicable>
These excludes were introduced in
open-telemetry#33192
I've opened a PR in core for this issue as well:
open-telemetry/opentelemetry-collector#10411
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-cd CI, CD, testing, build issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants