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

[CI/CD] Ping code owners does not work on some components that share base names #29571

Closed
crobert-1 opened this issue Nov 29, 2023 · 2 comments · Fixed by #29572
Closed

[CI/CD] Ping code owners does not work on some components that share base names #29571

crobert-1 opened this issue Nov 29, 2023 · 2 comments · Fixed by #29572
Assignees
Labels
bug Something isn't working ci-cd CI, CD, testing, build issues github_actions Pull requests that update Github_actions code

Comments

@crobert-1
Copy link
Member

crobert-1 commented Nov 29, 2023

Component(s)

No response

Describe the issue you're reporting

Problem

Our current workflow requires github-actions to ping code owners adding labels to issues and PRs. However, there's a bug currently that is hit for a label of a base component when there are multiple sub-components that share the same name, but the base name does not end in its component type.

Places where no pings are happening but should be:
#29082
#18491
#27046
(There are more)

Example Components

The extension/encoding label will not ping code owners, but the rest will. So in the terminology I'm using, extension/encoding is the base component, extension/encoding/zipkinencodingextension would be a sub-component, and the component type is extension.

extension/encoding/                                                     @open-telemetry/collector-contrib-approvers @atoulme @dao-jun @dmitryax @MovieStoreGuy @VihasMakwana
extension/encoding/jaegerencodingextension/                             @open-telemetry/collector-contrib-approvers @MovieStoreGuy @atoulme
extension/encoding/jsonlogencodingextension/                            @open-telemetry/collector-contrib-approvers @VihasMakwana @atoulme
extension/encoding/otlpencodingextension/                               @open-telemetry/collector-contrib-approvers @dao-jun @VihasMakwana
extension/encoding/textencodingextension/                               @open-telemetry/collector-contrib-approvers @MovieStoreGuy @atoulme
extension/encoding/zipkinencodingextension/                             @open-telemetry/collector-contrib-approvers @MovieStoreGuy @dao-jun

The extension/observer label will not ping code owners, but the rest will.

extension/observer/                                                     @open-telemetry/collector-contrib-approvers @dmitryax @rmfitzpatrick
extension/observer/dockerobserver/                                      @open-telemetry/collector-contrib-approvers @MovieStoreGuy
extension/observer/ecsobserver/                                         @open-telemetry/collector-contrib-approvers @dmitryax @rmfitzpatrick
extension/observer/ecstaskobserver/                                     @open-telemetry/collector-contrib-approvers @rmfitzpatrick
extension/observer/hostobserver/                                        @open-telemetry/collector-contrib-approvers @MovieStoreGuy
extension/observer/k8sobserver/                                         @open-telemetry/collector-contrib-approvers @rmfitzpatrick @dmitryax

Problem code

The problem is in get-codeowners.sh here:

# there may be more than 1 component matching a label
# if so, try to narrow things down by appending the component
# type to the label
if [[ ${RESULT} != 1 ]]; then
    COMPONENT_TYPE=$(echo "${COMPONENT}" | cut -f 1 -d '/')
    COMPONENT="${COMPONENT}${COMPONENT_TYPE}"
fi

RESULT is the number of lines matched by grepping the component passed in. In the case of COMPONENT=extension/encoding, RESULT is set to 6. The script then appends the COMPONENT_TYPE, extension in this case, to the COMPONENT and attempts to parse code owners from this component. However, there's no COMPONENT in the code owners file extension/encodingextension. This results in no pings being sent and the script ends "successfully"

This code is in place for some components that have labels that don't match their code owner entry name. One example is the resource detection processor. The label is processor/resourcedetection, but the code owners line is processor/resourcedetectionprocessor.

Solution

There are a few possible solutions here:

  1. Add another case in get-codeowners.sh that simply appends a / to the original component, before appending component type. Check if there are any results for this component, if so, return code owners as we know we matched the full path. In my given examples, the script would then search the CODEOWNERS file for extension/encoding/ or processor/resourcedetection/. At worst, no owners would be returned. Otherwise, it would properly find the entry and return owners in a determinate way. This seems like the best solution.
  2. Remove the functionality of appending COMPONENT_TYPE entirely. This would technically work today for all components as all of the base components are listed before their sub-components. This would work because the get-codeowners.sh script returns the first line in the file that matches the COMPONENT. The downside here is it would be order dependent in the CODEOWNERS file. This would introduce some level of instability though, so it's not a perfect solution. This would not work as processor/resourcedetectionprocessor is before processor/resourceprocessor alphabetically. If we removed append, the label processor/resource would ping processor/resourcedetection code owners incorrectly, as it comes before the resource processor alphabetically.
  3. Rename base components to end with their type. I don't think this is a good solution as there's no requirement that I'm seeing that requires components to end in their type. If components are required to end in their type, we should fix component directories and names which would be a big impacting and breaking change, and it should be done in its own issue.
@crobert-1 crobert-1 added needs triage New item requiring triage bug Something isn't working ci-cd CI, CD, testing, build issues github_actions Pull requests that update Github_actions code labels Nov 29, 2023
@crobert-1
Copy link
Member Author

I'll work on implementing solution 1.

@crobert-1 crobert-1 self-assigned this Nov 29, 2023
@atoulme atoulme removed the needs triage New item requiring triage label Nov 30, 2023
evan-bradley pushed a commit that referenced this issue Nov 30, 2023
…onents (#29572)

### Description:

**Existing bug:**
This resolves a bug in pinging code owners that was hit when a base
component doesn't end with its type, but has multiple sub-components.

For example, when adding the label `extension/encoding` to issues code
owners weren't being pinged. This is because there are multiple
sub-components (e.g. `extension/encoding/jaegarencodingextension`,
`extension/encoding/zipkinencodingextension`, etc), and
`extension/encoding` doesn't end with its own type `extension`.

**Solution:**
The solution to this bug is to also check if the original component can
be found if a single `/` is appended to it. This finds the component in
a determinate way and allows code owners to be pinged properly.

### Link to tracking Issue:
Resolves #29571

### Testing:
```
crobert$ ~/dev/opentelemetry-collector-contrib $ export COMPONENT=extension/encoding
crobert$ ~/dev/opentelemetry-collector-contrib $ .github/workflows/scripts/get-codeowners.sh 
@atoulme @dao-jun @dmitryax @MovieStoreGuy @VihasMakwana
crobert$ ~/dev/opentelemetry-collector-contrib $ export COMPONENT=extension/encoding/zipkinencoding
crobert$ ~/dev/opentelemetry-collector-contrib $ .github/workflows/scripts/get-codeowners.sh 
@MovieStoreGuy @dao-jun
crobert$ ~/dev/opentelemetry-collector-contrib $ export COMPONENT=processor/resourcedetection
crobert$ ~/dev/opentelemetry-collector-contrib $ .github/workflows/scripts/get-codeowners.sh 
@Aneurysm9 @dashpole
crobert$ ~/dev/opentelemetry-collector-contrib $ export COMPONENT=extension/observer
crobert$ ~/dev/opentelemetry-collector-contrib $ .github/workflows/scripts/get-codeowners.sh
@dmitryax @rmfitzpatrick
crobert$ ~/dev/opentelemetry-collector-contrib $ export COMPONENT=processor/resource
crobert$ ~/dev/opentelemetry-collector-contrib $ .github/workflows/scripts/get-codeowners.sh
@dmitryax
```
Previously, `extension/observer` and `extension/encoding` returned
nothing, which was the bug this resolves.
@crobert-1
Copy link
Member Author

Seems to be working: #29082 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ci-cd CI, CD, testing, build issues github_actions Pull requests that update Github_actions code
Projects
None yet
2 participants