Upgrade Elixir and OTP in GitHub Workflow #254

wingyplus · 2021-05-31T02:22:17Z

This change do:

Add Elixir 1.12.1 and upgrade 1.11 to latest release.
Add OTP 24.0.1 and upgrade major OTP release to latest release.
Use rebar3 v1.15.2 for OTP 21. And v1.16.1 for OTP 22+.

codecov · 2021-05-31T11:58:55Z

Codecov Report

Merging #254 (fb59106) into main (f14066e) will increase coverage by 0.08%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #254      +/-   ##
==========================================
+ Coverage   36.26%   36.34%   +0.08%     
==========================================
  Files          37       37              
  Lines        3157     3161       +4     
==========================================
+ Hits         1145     1149       +4     
  Misses       2012     2012

Flag	Coverage Δ
api	`63.01% <ø> (ø)`
elixir	`16.19% <ø> (ø)`
erlang	`36.34% <ø> (+0.08%)`	⬆️
exporter	`19.58% <ø> (ø)`
sdk	`78.89% <ø> (+0.15%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
apps/opentelemetry/src/otel_sampler.erl	`87.75% <0.00%> (+1.08%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f14066e...fb59106. Read the comment docs.

hauleth · 2021-05-31T13:22:25Z

Why remove OTP 21?

hauleth · 2021-05-31T13:23:40Z

.github/workflows/elixir.yml

@@ -33,8 +33,8 @@ jobs:
    name: Test on Elixir ${{ matrix.elixir_version }} (OTP ${{ matrix.otp_version }}) and ${{ matrix.os }}
    strategy:
      matrix:
-        otp_version: ['23.1', '22.3.4.2', '21.3.8.16']
-        elixir: ['1.11.1']
+        otp_version: ['24.0.1', '23.3.4.1', '22.3.4.19']


As we are supporting Elixir 1.11 we shouldn't remove OTP 21 from matrix, just ignore such set of options.

wingyplus · 2021-05-31T13:26:50Z

Why remove OTP 21?

The discussion is in #253. And Elixir 1.12 requires to run on OTP 22+. That’s why I decide to drop it. Or do you have any suggestions?

hauleth · 2021-05-31T13:33:57Z

We can exclude only Elixir 1.12 and OTP 21 and still run them - https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#example-excluding-configurations-from-a-matrix

wingyplus · 2021-05-31T13:39:59Z

We can exclude only Elixir 1.12 and OTP 21 and still run them - https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#example-excluding-configurations-from-a-matrix

@hauleth Thanks for your suggestion. Do you mind if I upgrade to 21.3.8.23?

hauleth · 2021-05-31T13:40:41Z

Not at all.

This change do: * Add Elixir 1.12.1 and upgrade 1.11 to latest release. * Add OTP 24.0.1 and upgrade major OTP release to latest release.

wingyplus · 2021-05-31T13:47:39Z

The OTP 21 is added back to the CI on both Elixir and OTP workflow. Please kindly review.

wingyplus · 2021-06-01T01:32:40Z

I’ve found similar issue with #253. Not sure why it happens on otp21

escript: exception error: undefined function rebar3:main/1
  in function  escript:run/2 (escript.erl, line 758)
  in call from escript:start/1 (escript.erl, line 277)
  in call from init:start_em/1 
  in call from init:do_boot/3 
Error: Process completed with exit code 127.

tsloughter · 2021-06-01T11:49:37Z

I thought it was because of rebar3 only being built on recent otp's now (otp-24 dropped deprecated stuff so couldn't use older builds with it). But I Thought 21 was new enough...

@ferd ?

ferd · 2021-06-01T11:54:23Z

We used the opportunity of clamping down on the rebar3 vs OTP compatibility window to just line up directly with what the OTP team supports all at once rather than doing it progressively, with the rationale that OTP24's JIT would provide enough of an incentive.

Versions 3.16.x are 22..24 so far.

We can still build and compile on 21, but that's not what ships pre-built.

wingyplus · 2021-06-01T12:13:08Z

@ferd i see in you mentioned in the blog (https://ferd.ca/you-ve-got-to-upgrade-rebar3.html) that 3.15.2 support OTP 19-23. Can we use that one?

I just see that setup-beam allow us to set rebar3 version (https://github.com/erlef/setup-beam#matrix-example-rebar3). So if we want to keep otp21, we might set the rebar3 to 3.15.2 to the pipeline and use the latest version for otp 22+.

And I notice that erlef change to setup-beam. Do you need me to change it instead of setup-elixir?

ferd · 2021-06-01T12:17:03Z

There's no reason it wouldn't work to use 3.15.2 for OTP-21 specifically. We cut the security release on that one specifically to still provide support as far back as posslbe (and eventually added 13.3.x with user contribs as well for people either needing older behaviour or OTP-18).

For a while this will be plenty, but over time the differences may accumulate and cause divergences.

tsloughter · 2021-06-01T13:38:38Z

Aaah, 22, not 21.

rebar3 v3.16.1 ship support OTP 22+, this cause ci fail on OTP 21. Fixes by using rebar3 v3.15.2 with OTP 21 and add it with include matrix instead. In this changes, upgrade OTP 24 from 24.0.1 to 24.0.2 and change setup-elixir to setup-beam instead.

wingyplus · 2021-06-01T14:22:46Z

@tsloughter @hauleth I have a fix for OTP 21 issue by using OTP 21 with rebar3 3.15.2 instead. Please kindly review.

ferd · 2021-06-01T20:57:28Z

Is it normal that the 4 checks that passed don't contain Erlang?

Looking at the checks I can see the following is broken for Erlang: https://github.com/open-telemetry/opentelemetry-erlang/actions/runs/896362703

And for Elixir:

I think we can't merge this, it appears the CI is broken but still showing as 4 successful checks.

ferd

The CI steps appear to still be broken.

wingyplus · 2021-06-02T00:30:34Z

Umm, if i understand it correctly. The run-on empty because I forget adding os into the include.

Let me investigate.

The ci will pass with error empty value on `runs-on`. The root cause is the field `os` needs to add to matrix.include. This change fix this issue.

wingyplus · 2021-06-02T02:50:48Z

I've added os into the include matrix. This should fix the problem. Can we trigger the pipeline?

ferd · 2021-06-02T12:56:36Z

Done, it seems to be running them all now. It seems to be failing about some of the pre-compiled module issues that are common across versions.

wingyplus · 2021-06-02T13:36:13Z

@ferd Thanks for your help. It's because I accidentally change Elixir to 1.12 in Erlang dialyzer which does not require since change to setup-beam. And I found the matrix.elixir variable is inconsistent, which cause the wrong test name.

All of this should fixed in latest changes.

ferd

Everything seems to work successfully now, let's get this merged!

wingyplus · 2021-06-02T13:38:15Z

@hauleth @ferd @tsloughter Thanks for your help. :)

hauleth · 2021-06-02T17:50:06Z

💚 💙 💜 💛 ❤️

wingyplus requested a review from a team May 31, 2021 02:22

github-actions bot added the scope-ci label May 31, 2021

wingyplus force-pushed the ci branch from 3539f36 to 5b181f6 Compare May 31, 2021 02:22

wingyplus changed the title ~~Upgrade Elixir and OTP~~ Upgrade Elixir and OTP in GitHub Workflow May 31, 2021

wingyplus mentioned this pull request May 31, 2021

Remove unused header in otel_span_ets #253

Merged

hauleth approved these changes May 31, 2021

View reviewed changes

hauleth requested changes May 31, 2021

View reviewed changes

Upgrade Elixir and OTP in GitHub Workflow

d0bfa1b

This change do: * Add Elixir 1.12.1 and upgrade 1.11 to latest release. * Add OTP 24.0.1 and upgrade major OTP release to latest release.

wingyplus force-pushed the ci branch from 5b181f6 to d0bfa1b Compare May 31, 2021 13:46

wingyplus requested a review from hauleth May 31, 2021 15:26

hauleth approved these changes May 31, 2021

View reviewed changes

Upgrade OTP 21,22,23

92009f2

hauleth approved these changes Jun 1, 2021

View reviewed changes

ferd requested changes Jun 1, 2021

View reviewed changes

Add os to matrix.include

432f7de

The ci will pass with error empty value on `runs-on`. The root cause is the field `os` needs to add to matrix.include. This change fix this issue.

thanabodee-c added 2 commits June 2, 2021 20:29

Fix build fail issues

7194b7f

Merge branch 'ci-2' into ci

fb59106

ferd approved these changes Jun 2, 2021

View reviewed changes

ferd merged commit 896be9c into open-telemetry:main Jun 2, 2021

wingyplus deleted the ci branch June 2, 2021 13:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade Elixir and OTP in GitHub Workflow #254

Upgrade Elixir and OTP in GitHub Workflow #254

wingyplus commented May 31, 2021 •

edited

Loading

codecov bot commented May 31, 2021 •

edited

Loading

hauleth commented May 31, 2021

hauleth May 31, 2021

wingyplus commented May 31, 2021 •

edited

Loading

hauleth commented May 31, 2021

wingyplus commented May 31, 2021

hauleth commented May 31, 2021

wingyplus commented May 31, 2021

wingyplus commented Jun 1, 2021 •

edited

Loading

tsloughter commented Jun 1, 2021

ferd commented Jun 1, 2021 •

edited

Loading

wingyplus commented Jun 1, 2021

ferd commented Jun 1, 2021

tsloughter commented Jun 1, 2021

wingyplus commented Jun 1, 2021

ferd commented Jun 1, 2021

ferd left a comment

wingyplus commented Jun 2, 2021

wingyplus commented Jun 2, 2021 •

edited

Loading

ferd commented Jun 2, 2021 •

edited

Loading

wingyplus commented Jun 2, 2021

ferd left a comment

wingyplus commented Jun 2, 2021

hauleth commented Jun 2, 2021

Upgrade Elixir and OTP in GitHub Workflow #254

Upgrade Elixir and OTP in GitHub Workflow #254

Conversation

wingyplus commented May 31, 2021 • edited Loading

codecov bot commented May 31, 2021 • edited Loading

Codecov Report

hauleth commented May 31, 2021

hauleth May 31, 2021

Choose a reason for hiding this comment

wingyplus commented May 31, 2021 • edited Loading

hauleth commented May 31, 2021

wingyplus commented May 31, 2021

hauleth commented May 31, 2021

wingyplus commented May 31, 2021

wingyplus commented Jun 1, 2021 • edited Loading

tsloughter commented Jun 1, 2021

ferd commented Jun 1, 2021 • edited Loading

wingyplus commented Jun 1, 2021

ferd commented Jun 1, 2021

tsloughter commented Jun 1, 2021

wingyplus commented Jun 1, 2021

ferd commented Jun 1, 2021

ferd left a comment

Choose a reason for hiding this comment

wingyplus commented Jun 2, 2021

wingyplus commented Jun 2, 2021 • edited Loading

ferd commented Jun 2, 2021 • edited Loading

wingyplus commented Jun 2, 2021

ferd left a comment

Choose a reason for hiding this comment

wingyplus commented Jun 2, 2021

hauleth commented Jun 2, 2021

wingyplus commented May 31, 2021 •

edited

Loading

codecov bot commented May 31, 2021 •

edited

Loading

wingyplus commented May 31, 2021 •

edited

Loading

wingyplus commented Jun 1, 2021 •

edited

Loading

ferd commented Jun 1, 2021 •

edited

Loading

wingyplus commented Jun 2, 2021 •

edited

Loading

ferd commented Jun 2, 2021 •

edited

Loading