-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Remove path parts from component label suffixes #38527
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
Comments
I came up with the following process to generate shorter label names. All components would have labels under the 50-character limit if we applied this. LABEL_NAME=$(echo "${COMPONENT}" | sed -E 's%^(.+)/(.+)\1%\1/\2%')
OIFS=${IFS}
IFS='/'
for SEGMENT in ${COMPONENT}
do
# If a component is named pkg/mypackage and the segment is mypackage, it will always match. Skip the last part of the path.
r="/${SEGMENT}\$"
if [[ "${COMPONENT}" =~ ${r} ]]; then
break
fi
LABEL_NAME=$(echo "${LABEL_NAME}" | sed -E "s%^(.+)${SEGMENT}\$%\1%")
done
IFS=${OIFS}
if (( "${#LABEL_NAME}" > 50 )); then
echo "'${LABEL_NAME}' exceeds GitHubs 50-character limit on labels, skipping"
continue
fi However, while this handles the path --> label conversion, we need something afterward that can do label --> path, which is much harder since our path names have inconsistencies that make it basically impossible to determine which parts of the path were removed from the label. I think our best bet will be to change label generation from an on-demand process to something that is generated and checked in to the repo. If we have a file that contains each path and its corresponding label on each line, we could do a lookup when converting in either direction. |
@evan-bradley, I am new to Go but confident I can give this a good shot. Can I get it assigned to me? |
/assign |
Sure thing @PhilemonBrain. This will almost entirely consist of working in Bash/GitHub Actions/Makefiles, so no Go knowledge is necessary. 🙂 You'll want to take a look at the following:
Thanks in advance for looking at this! |
I would love to work on this, however, I sincerely think this might be a bit too much for me 🥲. Thanks @evan-bradley. I would pick another issue |
No problem, thanks for your interest! |
@evan-bradley I would love to work on this. Could you please assign it to me? |
Can do. Thanks for looking into it! |
Thanks for fixing this @gabgg71! |
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description To generate labels for all components, including those with paths longer than 50 characters: In the root Makefile, a phony target is created to generate the .github/component_labels file, which contains the component paths and their corresponding labels (shortened paths for those exceeding 50 characters by removing repeated patterns in the string). The file is space-delimited. In the build-and-test workflow, under the checks job, a step is added to verify that the .github/component_labels file exists. The scripts used in the workflows are adjusted to use the .github/component_labels file as a reference for mappings between component paths and their labels. <!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. --> #### Link to tracking issue Fixes open-telemetry#38527
Uh oh!
There was an error while loading. Please reload this page.
Component(s)
No response
Is your feature request related to a problem? Please describe.
In our "create component labels" script we have a few components that are just barely over GitHub's 50 character limit for labels. From a recent workflow run:
Describe the solution you'd like
For each of these, there are suffixes in the last part of the path that can be removed since the suffix is already present in the path and therefore it's clear what type of component it is.
So
extension/encoding/awscloudwatchmetricstreamsencoding
could becomeextension/encoding/awscloudwatchmetricstreams
andreceiver/hostmetrics/internal/scraper/filesystemscraper
could becomereceiver/hostmetrics/internal/scraper/filesystem
.This would get us under the 50-character limit for now and wouldn't require too many changes to how the script operates.
Describe alternatives you've considered
Support a field in mdatagen that allows us to manually shorten these, then generate the authoritative list of labels from mdatagen. The 50-character limit could be enforced during the generation step.
Additional context
No response
The text was updated successfully, but these errors were encountered: