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

Add daily semconv check #5980

Merged
merged 5 commits into from
Jan 21, 2025
Merged

Conversation

trask
Copy link
Member

@trask trask commented Jan 18, 2025

Runs build-dev.yml once a day and opens an issue in the semantic convention repository on failure.

cc @open-telemetry/specs-semconv-approvers

@trask trask requested a review from a team as a code owner January 18, 2025 22:38
@meswapnilk
Copy link
Contributor

meswapnilk commented Jan 20, 2025

@trask you may want to check the prettier check locally and update the PR?

@svrnm
Copy link
Member

svrnm commented Jan 20, 2025

Thanks @trask!

@chalin please take a look, you know that build dev workflow best

@svrnm svrnm added the CI/infra CI & infrastructure label Jan 20, 2025
Copy link
Contributor

@chalin chalin left a comment

Choose a reason for hiding this comment

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

Very cool.

I get the feeling that this workflow would be best kept in the semconv repo. WDYT?

@trask
Copy link
Member Author

trask commented Jan 20, 2025

I get the feeling that this workflow would be best kept in the semconv repo. WDYT?

I'm not sure how to run the docs workflow from the semconv repo (maybe could submodule it in?)

@chalin
Copy link
Contributor

chalin commented Jan 20, 2025

I'm not sure how to run the docs workflow from the semconv repo (maybe could submodule it in?)

@svrnm once mentioned that it is possible to reference/call workflows from other repos. Is that right?

@trask
Copy link
Member Author

trask commented Jan 20, 2025

@svrnm once mentioned that it is possible to reference/call workflows from other repos. Is that right?

yeah, what I didn't consider though is that the (semconv) workflow could directly check out the docs repo (no need to submodule), I'll try this out, thanks!

@trask
Copy link
Member Author

trask commented Jan 21, 2025

yeah, what I didn't consider though is that the (semconv) workflow could directly check out the docs repo (no need to submodule), I'll try this out, thanks!

I think the problem is this:

If you reuse a workflow from a different repository, any actions in the called workflow run as if they were part of the caller workflow. For example, if the called workflow uses actions/checkout, the action checks out the contents of the repository that hosts the caller workflow, not the called workflow.

I do think we could copy build-dev.yml from this repo into the semconv repo, and update the actions/checkout steps to check out the opentelemetry.io repo instead of the semconv repo.

I guess I see this as an integration test for the website repo, and so it doesn't seem that out-of-place to live in the website repo.

Maybe a better option instead of opening the issue in semconv repo is to open the issue in the website repo but tag the semconv maintainers on it?

@chalin
Copy link
Contributor

chalin commented Jan 21, 2025

Ok, given all that, I'd be opening to having the workflow run in this repo. I'd rather have the issue created in open-telemetry/semantic-conventions (at least for now).

@chalin
Copy link
Contributor

chalin commented Jan 21, 2025

/fix:format

@opentelemetrybot
Copy link
Collaborator

You triggered fix:format action run at https://github.com/open-telemetry/opentelemetry.io/actions/runs/12879013379

@opentelemetrybot
Copy link
Collaborator

fix:format failed or was cancelled. For details, see https://github.com/open-telemetry/opentelemetry.io/actions/runs/12879013379.

@chalin
Copy link
Contributor

chalin commented Jan 21, 2025

fix:format failed or was cancelled. For details, see https://github.com/open-telemetry/opentelemetry.io/actions/runs/12879013379.

Our bot doesn't have the permission to push updates to your workflow files:

To https://github.com/trask/opentelemetry.io.git
 ! [remote rejected]   HEAD -> daily-semconv-check (refusing to allow a GitHub App to create or update workflow `.github/workflows/build-semconv-daily.yml` without `workflows` permission)
error: failed to push some refs to 'https://github.com/trask/opentelemetry.io.git'

@trask, you should update from main, and run npm run fix:format locally before repushing.

@trask
Copy link
Member Author

trask commented Jan 21, 2025

/fix:format

@opentelemetrybot
Copy link
Collaborator

You triggered fix:format action run at https://github.com/open-telemetry/opentelemetry.io/actions/runs/12879782312

@opentelemetrybot
Copy link
Collaborator

fix:format failed or was cancelled. For details, see https://github.com/open-telemetry/opentelemetry.io/actions/runs/12879782312.

@trask
Copy link
Member Author

trask commented Jan 21, 2025

@trask, you should update from main, and run npm run fix:format locally before repushing.

npm run fix:format fails locally for me (maybe because Windows?)

npm error code EINVALIDTAGNAME
npm error Invalid tag name "prettier${PRETTIER_AT_VERS}" of package "prettier$
{PRETTIER_AT_VERS}": Tags may not have any characters that encodeURIComponent encodes.

@trask trask mentioned this pull request Jan 21, 2025
@trask
Copy link
Member Author

trask commented Jan 21, 2025

Replaced by #5996

@trask trask closed this Jan 21, 2025
@trask
Copy link
Member Author

trask commented Jan 21, 2025

new plan, I'm going to move the yml file out of workflows, then /fix:format then move it back 😅

@trask trask reopened this Jan 21, 2025
@trask
Copy link
Member Author

trask commented Jan 21, 2025

/fix:format

@opentelemetrybot
Copy link
Collaborator

You triggered fix:format action run at https://github.com/open-telemetry/opentelemetry.io/actions/runs/12879883331

@opentelemetrybot
Copy link
Collaborator

fix:format failed or was cancelled. For details, see https://github.com/open-telemetry/opentelemetry.io/actions/runs/12879883331.

@trask
Copy link
Member Author

trask commented Jan 21, 2025

/fix:format

@opentelemetrybot
Copy link
Collaborator

You triggered fix:format action run at https://github.com/open-telemetry/opentelemetry.io/actions/runs/12879900677

@opentelemetrybot
Copy link
Collaborator

fix:format was successful.

IMPORTANT: (RE-)RUN /fix:all to ensure that there are no remaining check issues.

@chalin chalin force-pushed the daily-semconv-check branch from f3f1f31 to 1c64b2a Compare January 21, 2025 09:15
Copy link
Contributor

@chalin chalin left a comment

Choose a reason for hiding this comment

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

Let's give this a try. I'm thinking that we might want to trigger on commits to semconv main instead of a daily cron job, but we can tweak that later if needed.

@chalin chalin merged commit 208cca7 into open-telemetry:main Jan 21, 2025
17 checks passed
@chalin
Copy link
Contributor

chalin commented Jan 21, 2025

new plan, I'm going to move the yml file out of workflows, then /fix:format then move it back 😅

😅 ... a lot of work, but it did the trick! 🙌🏻

Next time maybe consider locally (and temporarily) replacing npx prettier${PRETTIER_AT_VERS} by npx prettier@latest.

npm run fix:format fails locally for me (maybe because Windows?)

Right, we don't officially support builds from Windows. I've wanted to work on this in the past but I don't have a Windows environment to test from 🤷🏼‍♂️.

@trask
Copy link
Member Author

trask commented Jan 22, 2025

Next time maybe consider locally (and temporarily) replacing npx prettier${PRETTIER_AT_VERS} by npx prettier@latest.

this worked! thanks

@trask trask deleted the daily-semconv-check branch January 22, 2025 23:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/infra CI & infrastructure
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants