oolong-dev / OpenTelemetry.jl

An unofficial implementation of OpenTelemetry in Julia.
https://oolong.dev/OpenTelemetry.jl/
Apache License 2.0
30 stars 9 forks source link

fix: switch off precompilation for OpenTelemetrySDK in Julia 1.10+ #94

Closed pankgeorg closed 8 months ago

pankgeorg commented 1 year ago

"fixes": https://github.com/oolong-dev/OpenTelemetry.jl/issues/93

~This moves the type piracy of the Base.schedule at runtime, to provide compatibility with Julia 1.10.~

This disables precompilation for OpenTelemetrySDK for recent versions of Julia, as per https://docs.julialang.org/en/v1/manual/modules/#Module-initialization-and-precompilation

Sysimages will break!

The final fix will probably be something else, but I'm adding that here for raising awareness.

Tests pass, but I'm probably missing something (I'm learning about OpenTelemetry as we speak! super nice talk at JuliaCon btw 🤗)

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage has no change and project coverage change: +0.21% :tada:

Comparison is base (f2f3c1c) 76.30% compared to head (b7dda35) 76.51%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #94 +/- ## ========================================== + Coverage 76.30% 76.51% +0.21% ========================================== Files 47 47 Lines 1422 1422 ========================================== + Hits 1085 1088 +3 + Misses 337 334 -3 ``` | [Files Changed](https://app.codecov.io/gh/oolong-dev/OpenTelemetry.jl/pull/94?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oolong-dev) | Coverage Δ | | |---|---|---| | [src/sdk/src/OpenTelemetrySDK.jl](https://app.codecov.io/gh/oolong-dev/OpenTelemetry.jl/pull/94?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oolong-dev#diff-c3JjL3Nkay9zcmMvT3BlblRlbGVtZXRyeVNESy5qbA==) | `33.33% <ø> (ø)` | | | [src/sdk/src/patch.jl](https://app.codecov.io/gh/oolong-dev/OpenTelemetry.jl/pull/94?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oolong-dev#diff-c3JjL3Nkay9zcmMvcGF0Y2guamw=) | `100.00% <ø> (ø)` | | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/oolong-dev/OpenTelemetry.jl/pull/94/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=oolong-dev)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

pankgeorg commented 1 year ago

This is also a bit problematic: sysimage-type issue (with this PR)

ERROR: LoadError: InitError: Evaluation into the closed module `OpenTelemetrySDK` breaks incremental compilation because the side effects will not be permanent. This is likely due to some other module mutating `OpenTelemetrySDK` with `eval` during precompilation - don't do this.
Stacktrace:
  [1] eval
    @ Core ./boot.jl:383 [inlined]
  [2] committypepiracy()
    @ OpenTelemetrySDK ~/.julia/packages/OpenTelemetrySDK/BblRx/src/patch.jl:8
  [3] __init__()
    @ OpenTelemetrySDK ~/.julia/packages/OpenTelemetrySDK/BblRx/src/OpenTelemetrySDK.jl:21
  [4] run_module_init(mod::Module, i::Int64)
    @ Base ./loading.jl:1128
  [5] register_restored_modules(sv::Core.SimpleVector, pkg::Base.PkgId, path::String)
    @ Base ./loading.jl:1116
  [6] _include_from_serialized(pkg::Base.PkgId, path::String, ocachepath::String, depmods::Vector{Any})
    @ Base ./loading.jl:1061
  [7] _require_search_from_serialized(pkg::Base.PkgId, sourcepath::String, build_id::UInt128)
    @ Base ./loading.jl:1575
  [8] _require(pkg::Base.PkgId, env::String)
    @ Base ./loading.jl:1932
  [9] __require_prelocked(uuidkey::Base.PkgId, env::String)
    @ Base ./loading.jl:1806
 [10] #invoke_in_world#3
    @ Base ./essentials.jl:921 [inlined]
 [11] invoke_in_world
    @ Base ./essentials.jl:918 [inlined]
 [12] _require_prelocked(uuidkey::Base.PkgId, env::String)
    @ Base ./loading.jl:1797
 [13] macro expansion
    @ Base ./loading.jl:1784 [inlined]
 [14] macro expansion
    @ Base ./lock.jl:267 [inlined]
 [15] __require(into::Module, mod::Symbol)
    @ Base ./loading.jl:1747
 [16] #invoke_in_world#3
    @ Base ./essentials.jl:921 [inlined]
 [17] invoke_in_world
    @ Base ./essentials.jl:918 [inlined]
 [18] require(into::Module, mod::Symbol)
    @ Base ./loading.jl:1740
 [19] include
    @ Base ./Base.jl:489 [inlined]
 [20] include_package_for_output(pkg::Base.PkgId, input::String, depot_path::Vector{String}, dl_load_path::Vector{String}, load_path::Vector{String}, concrete_deps::Vector{Pair{Base.PkgId, UInt128}}, source::Nothing)
    @ Base ./loading.jl:2216
 [21] top-level scope
    @ stdin:3
during initialization of module OpenTelemetrySDK
in expression starting at /home/pgeorgakopoulos/.julia/packages/OpenTelemetry/CzLUY/src/OpenTelemetry.jl:1
in expression starting at stdin:3
krynju commented 8 months ago

schedule piracy removed in https://github.com/oolong-dev/OpenTelemetry.jl/pull/96