Appliscale / xprof

A visual tracer and profiler for Erlang and Elixir.
Other
275 stars 33 forks source link

Support both cowboy 1.x and 2.x version #99

Closed gomoripeti closed 3 years ago

gomoripeti commented 6 years ago

Why would we support cowboy 1.x

Handling cowboy dependency

Compiling

Runtime

To sum up there are probably two options

  1. Explicit
    • user defines XPROF_COWBOY_1 to enforce version at compile time
    • rebar.config.script + ifdefs in code
  2. Implicite/Automatice
    • mix.exs and rebar3 override to adjust dependency
    • automatice macro if maps are supported (+ifdefs in code)
    • xprof_gui_app detects verison at runtime

...and probably a third, simplest one I did not think of.

Sample WIP code for 1. can be seen here: xprof_gui_cowboy_handler.erl, xprof_gui_app.erl

afronski commented 6 years ago

3rd possible options would be split.

Let's call packages as following - xprof_core (matching, throttling, tracing etc.), xprof_api - which is cowboy 2.x wrapper, xprof_legacy_api - which is cowboy 1.x wrapper and xprof_front_end - which is UI package (with support for both REST APIs).

Then we can publish two packages on hex.pm:

Benefits:

Drawbacks:

gomoripeti commented 6 years ago

good suggestion

I am a bit scared of introducing so many hex packages and OTP applications (even for a javascript-only component) but I should get over this :) . Maybe we could generate multiple hex packages (xprof_api/xprof_legacy_api) from the same OTP application and code base. (These are basically 1 module apps)

I wonder if there is any existing example in the hex world for such fine-grained packaging?

afronski commented 6 years ago

@gomoripeti Yup, it is possible e.g. phoenix is organized in such way - https://hex.pm/packages/phoenix - phoenix_pubsub is there by default, also there is phoenix_html, phoenix_ecto in your generated application and so on. I wouldn't name that fine-grained, but it is really close to what we want to achieve here.

gomoripeti commented 6 years ago

just a note that if we have xprof_gui (depending on cowboy 2.0.0) and xprof_legacy_gui (depending on cowboy 1.1.2) in the same repo/umbrella project, rebar3 will only fetch one of the cowboy versions. (Obviously, to avoid version conflict)

===> Skipping cowboy (from {pkg,<<"cowboy">>,<<"2.0.0">>,
                                      <<"A3B680BCC1156C6FBCB398CC56ADC35177037012D7DC28D8F7E7926D6C243561">>}) as an app of the same name has already been fetched

scetching up deps hell: repo1 (compiles with any Erlang version)

apps/xprof_legacy
β”œβ”€ apps/xprof_core
└─ apps/xprof_legacy_gui (includes js and common rest module)
   └─ cowboy 1.1.2 (hex pkg)

repo2 (Erlang 19+ only):

apps/xprof
β”œβ”€ xprof_core (hex pkg)
└─ apps/xprof_gui (overrides cowboy version)
   β”œβ”€ cowboy 2.0.0 (hex pkg)
   └─ xprof_legacy_gui (hex pkg)
gomoripeti commented 6 years ago

question of naming and what should be the default

naming

  1. if the default is cowboy 2.x and OTP 19+ use _legacy suffix for old stuff
  2. if the default is cowboy 1.x and old OTP support use _c2 or _ng (or you name it) suffix for new stuff

(which cowboy version and hence which gui variant to use)

So it seems that cowboy 2.x is a special case, so the default should be the old one. On the other hand it's a good idea to have the new thing as default (in general; forward looking) as later other, new features could come also requiring eg. OTP 19+ (most notably the tracer NIF)

afronski commented 6 years ago

I'd vote for new stuff as a default. I know that will require more work for the older VMs, but that's the deal with supporting and using older stuff - if we're aiming into bigger adoption, the new should be our default.

ppikula commented 6 years ago

I am also for keeping new as default and hopefully dropping legacy stuff after some time.

gomoripeti commented 6 years ago

packaging and scenarios (maybe we don't want to support all of them)

Issues

  1. Mix does not support passing OS env when building rebar dependency before 1.5.3 (included in 1.6.0 - https://github.com/elixir-lang/elixir/pull/6933 )
  2. rebar3 does not support multiple versions like "1.1.2 or ~> 2.0.0" (big bummer) a version like {cowboy, "~> 2.0.0 ir 1.1.2"} in rebar.config cause crash like
    ===> Uncaught error: {badmatch,[<<"2.0.0">>,<<"or">>,<<"1.1.2">>]}
    ===> Stack trace to the error location:
    [{rebar_app_utils,update_source,3,
                     [{file,"/Users/peter/git/rebar3/_build/default/lib/rebar/src/rebar_app_utils.erl"},
                      {line,259}]},

    multiple version in hex package dependency causes the dependency to be simply skipped

    ===> [xprof_gui:2.0.0-prealpha.2], Bad dependency version for cowboy: 1.1.2 or ~> 2.0.
    ...
         └─ xprof_gui─2.0.0-prealpha.2 (hex package)
            └─ jsone─1.3.1 (hex package)