romul / newrelic.ex

NewRelic agent for Elixir
MIT License
64 stars 19 forks source link

Ecto/Phoenix aren't optional #14

Open jodyrodd opened 5 years ago

jodyrodd commented 5 years ago

The Ecto dependency in the mix file is listed as optional: https://github.com/romul/newrelic.ex/blob/master/mix.exs#L27

      {:ecto, ">= 2.0.0", optional: true},

However, it isn't. It is being explicitly referenced here: https://github.com/romul/newrelic.ex/blob/master/lib/new_relic/plug/instrumentation.ex#L44

 defp infer_model(%{__struct__: model_type, __meta__: %Ecto.Schema.Metadata{}}) do
    model_name(model_type)
  end

and other places in the file.

Without explicitly including Ecto, the project won't compile. Is this intentional? I'd rather not include Ecto in my project if I don't have to.

sb8244 commented 5 years ago

Thanks @jodyrodd. That would be a compile error. I think it could be fixed by checking the schema key at runtime, or by discarding the schema check altogether. I will investigate how it's being used.

jodyrodd commented 5 years ago

The optional phoenix include is also not optional and generates compile errors on projects that don't use it.