scoutapp / scout_apm_elixir

ScoutAPM Elixir Agent. Supports Phoenix and other frameworks.
https://scoutapm.com
Other
36 stars 20 forks source link

wrong syntax on docs #50

Closed prem-prakash closed 5 years ago

prem-prakash commented 6 years ago

the syntax bellow does not work

defmodule Searcher do
  use ScoutApm.Tracing

  # Time associated with this function will appear under "Hound" in timeseries charts.
  # The function will appear as `Hound/open_search` in transaction traces.
  @timing(category: "Hound")
  def open_search(url) do
    navigate_to(url)
  end

  # Time associated with this function will appear under "Hound" in timeseries charts.
  # The function will appear as `Hound/search` in transaction traces.
  @timing(category: "Hound", name: "search")
  def open_search(url) do
    navigate_to(url)
  end

tried on a module:

defmodule Apollo.Cache.MemcachedAdapter do
  @moduledoc """
  Wrapper around Memcache to allow timing instrumentation
  """
  use ScoutApm.Tracing

  alias Apollo.MemcachedWorker

  @timing category: "Memcached", name: "get"
  def get(key) do
    Memcache.get(MemcachedWorker, key)
  end

  @timing category: "Memcached", name: "set"
  def set(key, value) do
    Memcache.set(MemcachedWorker, key, value)
  end

  @timing category: "Memcached", name: "delete"
  def delete(key) do
    Memcache.delete(MemcachedWorker, key)
  end

  @timing category: "Memcached", name: "incr"
  def incr(key) do
    Memcache.incr(MemcachedWorker, key, default: 1, by: 1)
  end
end

the erlang VM starts to consume memory and the module became unresponsive

changing for this syntax it works fine

defmodule Apollo.Cache.MemcachedAdapter do
  @moduledoc """
  Wrapper around Memcache to allow timing instrumentation
  """
  import ScoutApm.Tracing

  alias Apollo.MemcachedWorker

  def get(key) do
    timing("Memcached", "get") do
      Memcache.get(MemcachedWorker, key)
    end
  end

  def set(key, value) do
    timing("Memcached", "set") do
      Memcache.set(MemcachedWorker, key, value)
    end
  end

  def delete(key) do
    timing("Memcached", "delete") do
      Memcache.delete(MemcachedWorker, key)
    end
  end

  def incr(key) do
    timing("Memcached", "incr") do
      Memcache.incr(MemcachedWorker, key, default: 1, by: 1)
    end
  end
end
cschneid commented 6 years ago

Thank you for the report. We've recently reworked some parts of custom instrumentation to play nicer. The function annotations were clever, but caused some issues, and the next release will provide a midpoint option for instrumentation. I'm glad you have the inside-the-function version of tracing working.

To clarify, the code compiled ok, but failed at runtime?

prem-prakash commented 6 years ago

yes the code compiled ok but failed at runtime by becoming unresponsive, or by changing the response of the functions, putting them inside a list with "do" as first item [do:, response]

cschneid commented 6 years ago

FYI - we're deprecating the @transaction and @timing approach to instrumentation, starting with the freshly released 0.4.1 version of the agent.

Now, prefer using a new macro called deftiming:

@timing_opts [category: "Memcached", name: "incr"]
deftiming incr(key) do
  # ...
end
itsderek23 commented 6 years ago

@cschneid - we need to update the help docs, correct?

http://help.apm.scoutapp.com/#timing-functions-and-blocks-of-code

jstr commented 6 years ago

It looks like both the docs linked above and those on Hex.pm (e.g. the code comments) are out of date with regard to the deprecations.

cschneid commented 6 years ago

@jstr - I just looked at our generated docs and it looks ok. We do still document the @transaction style, which we should further deemphasise, but the newer style deftransaction is there as the recommended version.

Can you point to what you're seeing?

mitchellhenke commented 5 years ago

The docs have been updated to reflect the deprecation of @timing in favor of the new implementation and style. If it is unclear, please let us know! 🙂