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

Integrate WebSocketLogger #131

Merged
merged 2 commits into from
Feb 20, 2019
Merged

Integrate WebSocketLogger #131

merged 2 commits into from
Feb 20, 2019

Conversation

hustf
Copy link
Collaborator

@hustf hustf commented Feb 17, 2019

This is the first attempt at loading a dedicated logger with WebSockets, and the added functionality would bump version number to 1.3.0 (1.2.0 added a number of 'show' methods').

The current logging implementation in Julia standard libraries can be used quite creatively (ref. Revise, Rebugger), and can possibly be fast as well. For applications with websockets, logging is especially helpful in determining the actual timing and bottlenecks between multiple threads of execution.

Logging utilities in this package has already seen some iterations and is mentioned in 23 issues. This PR refers issue #111. From inline doc:

Differences to stdlib/Logging/ConsoleLogger:

    - default timestamp on logging messages (except @info)
    - a 'shouldlog' function can be passed in. The `shouldlog_default` function filters
        on HTTP.Servers messages as well as on message_limits
    - :wslog => true flag which may be used for context-sensitive output
        from 'show' methods. This means a user can define 'show' methods
        which are used with this logger without affecting the behaviour
        defined in other modules.
    - :limited => true is included in the default IOContext. Keyword: show_limited
    - string_with_env_ws is exported for easy overloading on specific types
    - @info, @debug, @warn etc. will splat the first argument if it's a tuple arguments, e.g.

    julia> var = "a"
    "a"
    julia> @info (1, var)
    [ Info: 1a

modified: README.md WebSocketLogger and v1.3.0
new file: examples/count_with_logger.jl Logging async Websocket peers
modified: examples/minimal_server.jl Use WebSocketLogger
deleted: logutils/log_http.jl
deleted: logutils/logutils_ws.jl
new file: src/Logger/websocketlogger.jl stdlib/ConsoleLogger + /logutils.jl
modified: src/WebSockets.jl Include / export WebSocketLogger, @WSLog
modified: test/logformat.jl ConsoleLogger -> WebSocketLogger
modified: test/runtests.jl Include test_websoketlogger.jl
new file: test/test_websocketlogger.jl

new file:   examples/count_with_logger.jl Logging async Websocket peers
modified:   examples/minimal_server.jl Use WebSocketLogger
deleted:    logutils/log_http.jl
deleted:    logutils/logutils_ws.jl
new file:   src/Logger/websocketlogger.jl stdlib/ConsoleLogger + /logutils.jl
modified:   src/WebSockets.jl  Include / export WebSocketLogger, @WSLog
modified:   test/logformat.jl  ConsoleLogger -> WebSocketLogger
modified:   test/runtests.jl   Include test_websoketlogger.jl
new file:   test/test_websocketlogger.jl
@coveralls
Copy link

coveralls commented Feb 17, 2019

Coverage Status

Coverage increased (+1.2%) to 80.72% when pulling 8d3e6c0 on logging into 64dfe1d on master.

@hustf
Copy link
Collaborator Author

hustf commented Feb 18, 2019

@shashi, can you try comment on the below, or suggest someone with expertise on stdlib/Logging to do it?

'/examples/count_with_logger.jl' is surprisingly slow (0.5 seconds for 20 messages). I have not made a comparison as of yet. The somewhat drastic change compared to ConsoleLogger is the addition of a 'shouldlog' function field. Is that, for some reason I can't really see, bad for performance?

@hustf
Copy link
Collaborator Author

hustf commented Feb 20, 2019

As documented in #111, there's an apparent performance improvement. Merging this.

@hustf hustf merged commit 027514d into master Feb 20, 2019
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.

2 participants