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

Removing logging from production builds #26

Closed
rome-user opened this issue Mar 22, 2023 · 10 comments · Fixed by #27
Closed

Removing logging from production builds #26

rome-user opened this issue Mar 22, 2023 · 10 comments · Fixed by #27

Comments

@rome-user
Copy link
Contributor

In release builds I am noticing trace-level and debug-level log messages that I do not want in the compiled JS file. Mostly to save space because my code has a lot of trace-level and debug-level logs for development purposes.

I have taken a quick look at the macros like trace and debug. They ultimately return forms invoking a private log-expr function. Would it be possible to offer an option to disable logging entirely? This could be done within the logging macros themselves, or introducing closure constant lambdaisland.glogi/enabled then wrapping the body of log-expr like so.

(when ^boolean lambdaisland.glogi/enabled
  (do stuff))

which either goes away entirely or becomes (do stuff) under advanced optimizations.

@plexus
Copy link
Member

plexus commented Mar 22, 2023

This can be configured on the Google closure level. In your cljs compiler config:

:closure-defines {goog.DEBUG false
                  goog.debug.LOGGING_ENABLED false}

@rome-user
Copy link
Contributor Author

rome-user commented Mar 22, 2023

shadow-cljs by default will set goog.DEBUG to false. I have set goog.debug.LOGGING_ENABLED to false in release builds, but I still see all of my trace-level and debug-level log messages in the JS bundle. I am interested in getting rid of these maps and function calls from release builds

@plexus
Copy link
Member

plexus commented Mar 22, 2023

In that case I'm assuming the Google closure library introduced some breaking changes... Would be good to look into that so we can update our docs.

@plexus
Copy link
Member

plexus commented Mar 22, 2023

I don't see any relevant changes in google closure... this is still in there

goog.log.ENABLED = goog.define('goog.log.ENABLED', goog.debug.LOGGING_ENABLED);

and every logging call is guarded by if (goog.log.ENABLED) { ... }, which the google closure library will strip out if it's false and advanced optimizations are switched on. So there should be no logging code present in the final production build artifact.

We'll have to start with a minimal reproduction of the issue. @rome-user if you could help with that that would be really appreciated. Set up a small repo with only a dependency on glogi, configuration for a cljs build tool, and a single namespace with some logging calls, with instructions on how to compile and run.

@rome-user
Copy link
Contributor Author

rome-user commented Mar 22, 2023

In my case, I am successfully getting DCE with the following modification to log-expr

(defn- log-expr [form level keyvals]
  (let [keyvals-map (apply array-map keyvals)
        formatter (::formatter keyvals-map 'identity)]
    `(when ~(with-meta 'goog.debug.LOGGING_ENABLED {:tag 'boolean})
       (log ~(::logger keyvals-map (str *ns*))
            ~level
            (~formatter ~(-> keyvals-map
                             (dissoc ::logger)
                             (assoc :line (:line (meta form)))))
            ~(:exception keyvals-map)))))

But with the original log-expr function, I am seeing a bunch of maps in my compiled JS file that correspond to inputs to macros like log/debug. I will attempt making a small repository soon to demonstrate this.

@plexus
Copy link
Member

plexus commented Mar 22, 2023

Are you seeing logs in your console/stdout or not?

@plexus
Copy link
Member

plexus commented Mar 22, 2023

A PR for further minimizing what we emit when logging is disabled is certainly welcome!

@rome-user
Copy link
Contributor Author

Logs are not being printed. But in the compiled JS file I can see the strings that would be logged if I had e.g. installed the console logger. Basically, the data structures we use for logging do not get DCE'd unless I modify log-expr as above.

I am currently going to set up a small shadow-cljs project demonstrating this. Then I will upload the repo so you can take a look

@rome-user
Copy link
Contributor Author

@plexus Sorry for taking a long time. Here is the repository https://github.com/rome-user/glogi-demo

@plexus
Copy link
Member

plexus commented Mar 22, 2023

Sorry for misunderstanding your issue. In that case the behavior you are seeing is to be expected with the current implementation. A patch to elide logging code when LOGGING_ENABLED is false would be more than welcome!

rome-user added a commit to rome-user/glogi that referenced this issue Mar 22, 2023
Fixes lambdaisland#26. This allows code from macros like trace, debug, etc. to be
removed by GCC when the user has the compiler constant
goog.debug.LOGGING_ENABLED set to false.
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 a pull request may close this issue.

2 participants