Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

automatically call instrument upon every function definition #52

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cloojure
Copy link

Hi - We need a way to automatically call instrument after a function is defined. It is too error-prone to ask the user to include (st/instrument) at the bottom of each source-code file.

This way works, but I am open to suggestions for other implementations.

Of course, it may be best to put this variation in another NS, perhaps orchestra.auto or something.

Alan

@jeaye
Copy link
Owner

jeaye commented Aug 1, 2020

Hello! Firstly, thanks for not only using Orchestra, but for taking the time to suggest a change to it.

This topic is odd to me, since I've never thought about instrumentation this way. I've seen a few other people putting instrumentation calls at the bottom of their namespaces, but I figured that wasn't the most common approach.

Within my code, this never happens. Instead, I hook into the life cycle of the program and use that to instrument.

For example, in OkLetsPlay's front-end:

  1. We instrument when starting up the CLJS + React app
  2. We instrument in Figwheel's on-jsload callback, to catch new functions being written

In OkLetsPlay's back-end:

  1. We use integrant to manage the life cycle and instrument when starting a service
  2. When we hot reload, or reload via nrepl, instrumentation is applied again

To me, this sort of approach is preferred over Orchestra being opinionated about when to instrument. Especially since I know at OkLetsPlay we have various different flags, configs, and conditions which can result in instrumentation being enabled or disabled.

So, are there some hooks in your projects which would allow you to mimic this? Most Clojure/Script frameworks I've seen have some sort of hooking mechanism to know when they've reloaded. Though we use integrant, ring also supports this out of the box, for example.

@cloojure
Copy link
Author

cloojure commented Aug 1, 2020 via email

@jeaye
Copy link
Owner

jeaye commented Aug 1, 2020

I agree that the approach you suggested can work. However, I suggested an alternative, which is what my team uses, and I don't think you addressed it in your comment.

Are you able to just hook a call to (st/instrument) into the life cycle of your program so that you don't need to do this on a per-defn or per-ns level?

If you can, but you're not interested in it, will you please explain why? You mentioned that it used to work a certain way with Schema, but spec and Schema are quite different. Furthermore, the benefit of the life cycle hook is that your instrumentation call can likely just be in one or two places, rather than with every defn or every ns.

For a last point, I want to specifically address your testing setup with Schema and talk about how I do testing with Orchestra as a comparison. The testing setup looks something like this:

; Defined in some shared testing ns.
(defn with-instrumentation [f]
  (s/check-asserts true)
  (s.test/instrument)
  (f)
  (s.test/unstrument))

; Then test namespaces use something like this, at the top of the ns.
(use-fixtures :once with-instrumentation)

This gives control on a per-test or per-test-ns level, rather than on just a global level, as to whether or not to instrument. Furthermore, it groups instrumentation with unstrementation in a simple 1:1 relationship which would be more complicated if each defn were instrumented individually.

@cloojure
Copy link
Author

cloojure commented Aug 1, 2020 via email

@fuadsaud
Copy link

@jeaye what's your strategy for hooking it up into the nrepl lifecycle?

@jeaye
Copy link
Owner

jeaye commented Jan 18, 2022

In the front-end, you can hook into Figwheel's mounting function to run custom code on reload. Instrument there.

In the back-end, you could put it top-level in your app's top-most namespace. If you combine that with ring's reload middleware, or similar, it should run whenever you change any ns, since the top-most ns will depend on all others. You could also hook it into integrant, or similar, if you use that for your reloaded workflow.

I like putting this behind an environment variable, like so:

(defmacro environment []
  (if-not (empty? (:myapp-environment env))
    (:myapp-environment env)
    "dev"))

(defmacro if-dev [if-forms then-forms]
  `(if-fn ~#(= (environment) "dev") ~if-forms ~then-forms))

(defmacro when-dev [& if-forms]
  `(if-dev (do ~@if-forms) nil))

; Somewhere...
(when-dev
  (st/instrument))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants