-
Notifications
You must be signed in to change notification settings - Fork 2.8k
[chore][ping code owners] Fix multiple matching components #38844
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
atoulme
approved these changes
Mar 20, 2025
atoulme
pushed a commit
that referenced
this pull request
Apr 10, 2025
…cally when possible (#39145) <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description The logic for pinging code owners where multiple labels matched a given component returned the first in alphabetical order. This worked generally as the first alphabetical response was the minimal matching value, as shown in #38844. However, this breaks on the kafka receiver labels. The component labels are sorted in alphabetical order by the full component name. This means `receiver/kafkametricsreceiver` is before `receiver/kafkareceiver` in the list. When the `receiver/kafka` component is referenced, the `receiver/kafkametrics` label is returned as it's the first matching label in alphabetical order. This change is to try to match component and label identically. If there's no identical match, return the first label in alphabetical order. Existing logic has been broken out into a separate script to avoid duplicating the logic. <!-- Issue number (e.g. #1234) or full URL to issue, if applicable. --> #### Link to tracking issue Unexpected behavior was found in #39129. `receiver/kafka` was the component, the label `receiver/kafkametrics` was added incorrectly, and the code owners were pinged for `receiver/kafka` correctly. When I removed the `receiver/kafkametrics` label and added `receiver/kafka`, the code owners were pinged for `receiver/kafkametrics` incorrectly. <!--Describe what testing was performed and which tests were added.--> #### Testing ``` # Testing issue where bug was detected $ .github/workflows/scripts/ping-codeowners-on-new-issue.sh No related components were given Adding the following labels: receiver/kafka Pinging code owners: - receiver/kafka: @pavolloffay @MovieStoreGuy @axw See [Adding Labels via Comments](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/CONTRIBUTING.md#adding-labels-via-comments) if you do not have permissions to add labels yourself. # Testing other components $ export COMPONENT="os:windows" $ .github/workflows/scripts/get-label-from-component.sh $ export COMPONENT="receiver/hostmetrics" $ .github/workflows/scripts/get-label-from-component.sh receiver/hostmetrics $ export COMPONENT="receiver/kafka" $ .github/workflows/scripts/get-label-from-component.sh receiver/kafka $ export COMPONENT="receiver/kafkametrics" $ .github/workflows/scripts/get-label-from-component.sh receiver/kafkametrics $ export COMPONENT="extension/encoding" $ .github/workflows/scripts/get-label-from-component.sh extension/encoding $ export ISSUE=39129 $ COMPONENT="receiver/kafka" $ .github/workflows/scripts/ping-codeowners-issues.sh Pinging code owners for receiver/kafka: @pavolloffay @MovieStoreGuy @axw. See [Adding Labels via Comments](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/CONTRIBUTING.md#adding-labels-via-comments) if you do not have permissions to add labels yourself. For example, comment '/label priority:p2 -needs-triaged' to set the priority and remove the needs-triaged label. $ COMPONENT="receiver/kafkametrics" $ .github/workflows/scripts/ping-codeowners-issues.sh Pinging code owners for receiver/kafkametrics: @dmitryax. See [Adding Labels via Comments](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/CONTRIBUTING.md#adding-labels-via-comments) if you do not have permissions to add labels yourself. For example, comment '/label priority:p2 -needs-triaged' to set the priority and remove the needs-triaged label. ```
akshays-19
pushed a commit
to akshays-19/opentelemetry-collector-contrib
that referenced
this pull request
Apr 23, 2025
…cally when possible (open-telemetry#39145) <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description The logic for pinging code owners where multiple labels matched a given component returned the first in alphabetical order. This worked generally as the first alphabetical response was the minimal matching value, as shown in open-telemetry#38844. However, this breaks on the kafka receiver labels. The component labels are sorted in alphabetical order by the full component name. This means `receiver/kafkametricsreceiver` is before `receiver/kafkareceiver` in the list. When the `receiver/kafka` component is referenced, the `receiver/kafkametrics` label is returned as it's the first matching label in alphabetical order. This change is to try to match component and label identically. If there's no identical match, return the first label in alphabetical order. Existing logic has been broken out into a separate script to avoid duplicating the logic. <!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. --> #### Link to tracking issue Unexpected behavior was found in open-telemetry#39129. `receiver/kafka` was the component, the label `receiver/kafkametrics` was added incorrectly, and the code owners were pinged for `receiver/kafka` correctly. When I removed the `receiver/kafkametrics` label and added `receiver/kafka`, the code owners were pinged for `receiver/kafkametrics` incorrectly. <!--Describe what testing was performed and which tests were added.--> #### Testing ``` # Testing issue where bug was detected $ .github/workflows/scripts/ping-codeowners-on-new-issue.sh No related components were given Adding the following labels: receiver/kafka Pinging code owners: - receiver/kafka: @pavolloffay @MovieStoreGuy @axw See [Adding Labels via Comments](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/CONTRIBUTING.md#adding-labels-via-comments) if you do not have permissions to add labels yourself. # Testing other components $ export COMPONENT="os:windows" $ .github/workflows/scripts/get-label-from-component.sh $ export COMPONENT="receiver/hostmetrics" $ .github/workflows/scripts/get-label-from-component.sh receiver/hostmetrics $ export COMPONENT="receiver/kafka" $ .github/workflows/scripts/get-label-from-component.sh receiver/kafka $ export COMPONENT="receiver/kafkametrics" $ .github/workflows/scripts/get-label-from-component.sh receiver/kafkametrics $ export COMPONENT="extension/encoding" $ .github/workflows/scripts/get-label-from-component.sh extension/encoding $ export ISSUE=39129 $ COMPONENT="receiver/kafka" $ .github/workflows/scripts/ping-codeowners-issues.sh Pinging code owners for receiver/kafka: @pavolloffay @MovieStoreGuy @axw. See [Adding Labels via Comments](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/CONTRIBUTING.md#adding-labels-via-comments) if you do not have permissions to add labels yourself. For example, comment '/label priority:p2 -needs-triaged' to set the priority and remove the needs-triaged label. $ COMPONENT="receiver/kafkametrics" $ .github/workflows/scripts/ping-codeowners-issues.sh Pinging code owners for receiver/kafkametrics: @dmitryax. See [Adding Labels via Comments](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/CONTRIBUTING.md#adding-labels-via-comments) if you do not have permissions to add labels yourself. For example, comment '/label priority:p2 -needs-triaged' to set the priority and remove the needs-triaged label. ```
Fiery-Fenix
pushed a commit
to Fiery-Fenix/opentelemetry-collector-contrib
that referenced
this pull request
Apr 24, 2025
…metry#38844) <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description The current `awk` expression will return all matches, so if a component label is a substring of another component label, the resulting label will be all matching labels appended to each other. Example: open-telemetry#38780 The label wasn't originally added because the [found label was too long to add](https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/13938538170/job/39010977164). However, code owners were properly pinged. When I added the proper label, the [incorrect label name was detected](open-telemetry#38780 (comment)). When searching the `.github/component_labels.txt` file, the label we want to add is the first match that we find. This is the **minimally matching label**, since the file is in alphabetical order. There may be a better `awk` way to implement this logic, but `head -n 1` returns the first matching line, if any are found in the given text. <!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. --> #### Link to tracking issue Related open-telemetry#38622 <!--Describe what testing was performed and which tests were added.--> #### Testing **Before:** ``` crobert$ ~/dev/contrib/first $ COMPONENT="receiver/hostmetrics" crobert$ ~/dev/contrib/first $ awk -v path="${COMPONENT}" 'index($1, path) > 0 || index($2, path) > 0 {print $1}' .github/component_labels.txt receiver/hostmetricsreceiver receiver/hostmetricsreceiver/internal/scraper/cpuscraper receiver/hostmetricsreceiver/internal/scraper/diskscraper receiver/hostmetricsreceiver/internal/scraper/filesystemscraper receiver/hostmetricsreceiver/internal/scraper/loadscraper receiver/hostmetricsreceiver/internal/scraper/memoryscraper receiver/hostmetricsreceiver/internal/scraper/networkscraper receiver/hostmetricsreceiver/internal/scraper/pagingscraper receiver/hostmetricsreceiver/internal/scraper/processesscraper receiver/hostmetricsreceiver/internal/scraper/processscraper receiver/hostmetricsreceiver/internal/scraper/systemscraper crobert$ ~/dev/contrib/first $ COMPONENT="extension/encoding" crobert$ ~/dev/contrib/first $ awk -v path="${COMPONENT}" 'index($1, path) > 0 || index($2, path) > 0 {print $1}' .github/component_labels.txt extension/encoding extension/encoding/avrologencodingextension extension/encoding/awscloudwatchmetricstreamsencodingextension extension/encoding/awslogsencodingextension extension/encoding/googlecloudlogentryencodingextension extension/encoding/jaegerencodingextension extension/encoding/jsonlogencodingextension extension/encoding/otlpencodingextension extension/encoding/skywalkingencodingextension extension/encoding/textencodingextension extension/encoding/zipkinencodingextension ``` **After:** ``` crobert$ ~/dev/contrib/first $ COMPONENT="receiver/hostmetrics" crobert$ ~/dev/contrib/first $ awk -v path="${COMPONENT}" 'index($1, path) > 0 || index($2, path) > 0 {print $1}' .github/component_labels.txt | head -n 1 receiver/hostmetricsreceiver crobert$ ~/dev/contrib/first $ COMPONENT="extension/encoding" crobert$ ~/dev/contrib/first $ awk -v path="${COMPONENT}" 'index($1, path) > 0 || index($2, path) > 0 {print $1}' .github/component_labels.txt | head -n 1 extension/encoding
Fiery-Fenix
pushed a commit
to Fiery-Fenix/opentelemetry-collector-contrib
that referenced
this pull request
Apr 24, 2025
…cally when possible (open-telemetry#39145) <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description The logic for pinging code owners where multiple labels matched a given component returned the first in alphabetical order. This worked generally as the first alphabetical response was the minimal matching value, as shown in open-telemetry#38844. However, this breaks on the kafka receiver labels. The component labels are sorted in alphabetical order by the full component name. This means `receiver/kafkametricsreceiver` is before `receiver/kafkareceiver` in the list. When the `receiver/kafka` component is referenced, the `receiver/kafkametrics` label is returned as it's the first matching label in alphabetical order. This change is to try to match component and label identically. If there's no identical match, return the first label in alphabetical order. Existing logic has been broken out into a separate script to avoid duplicating the logic. <!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. --> #### Link to tracking issue Unexpected behavior was found in open-telemetry#39129. `receiver/kafka` was the component, the label `receiver/kafkametrics` was added incorrectly, and the code owners were pinged for `receiver/kafka` correctly. When I removed the `receiver/kafkametrics` label and added `receiver/kafka`, the code owners were pinged for `receiver/kafkametrics` incorrectly. <!--Describe what testing was performed and which tests were added.--> #### Testing ``` # Testing issue where bug was detected $ .github/workflows/scripts/ping-codeowners-on-new-issue.sh No related components were given Adding the following labels: receiver/kafka Pinging code owners: - receiver/kafka: @pavolloffay @MovieStoreGuy @axw See [Adding Labels via Comments](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/CONTRIBUTING.md#adding-labels-via-comments) if you do not have permissions to add labels yourself. # Testing other components $ export COMPONENT="os:windows" $ .github/workflows/scripts/get-label-from-component.sh $ export COMPONENT="receiver/hostmetrics" $ .github/workflows/scripts/get-label-from-component.sh receiver/hostmetrics $ export COMPONENT="receiver/kafka" $ .github/workflows/scripts/get-label-from-component.sh receiver/kafka $ export COMPONENT="receiver/kafkametrics" $ .github/workflows/scripts/get-label-from-component.sh receiver/kafkametrics $ export COMPONENT="extension/encoding" $ .github/workflows/scripts/get-label-from-component.sh extension/encoding $ export ISSUE=39129 $ COMPONENT="receiver/kafka" $ .github/workflows/scripts/ping-codeowners-issues.sh Pinging code owners for receiver/kafka: @pavolloffay @MovieStoreGuy @axw. See [Adding Labels via Comments](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/CONTRIBUTING.md#adding-labels-via-comments) if you do not have permissions to add labels yourself. For example, comment '/label priority:p2 -needs-triaged' to set the priority and remove the needs-triaged label. $ COMPONENT="receiver/kafkametrics" $ .github/workflows/scripts/ping-codeowners-issues.sh Pinging code owners for receiver/kafkametrics: @dmitryax. See [Adding Labels via Comments](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/CONTRIBUTING.md#adding-labels-via-comments) if you do not have permissions to add labels yourself. For example, comment '/label priority:p2 -needs-triaged' to set the priority and remove the needs-triaged label. ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
The current
awk
expression will return all matches, so if a component label is a substring of another component label, the resulting label will be all matching labels appended to each other.Example: #38780
The label wasn't originally added because the found label was too long to add. However, code owners were properly pinged. When I added the proper label, the incorrect label name was detected.
When searching the
.github/component_labels.txt
file, the label we want to add is the first match that we find. This is the minimally matching label, since the file is in alphabetical order. There may be a betterawk
way to implement this logic, buthead -n 1
returns the first matching line, if any are found in the given text.Link to tracking issue
Related #38622
Testing
Before:
After: