Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(741)

Issue 2046703002: Add 'flog' service implementation. (Closed)

Created:
4 years, 6 months ago by dalesat
Modified:
4 years, 6 months ago
Reviewers:
kulakowski
CC:
mojo-reviews_chromium.org, gregsimon, qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, abarth-chromium, Aaron Boodman, darin (slow to review), ben+mojo_chromium.org
Base URL:
https://github.com/domokit/mojo.git@master
Target Ref:
refs/heads/master
Project:
mojo
Visibility:
Public.

Description

Add 'flog' service implementation. This is the first take on the service implementation for formatted logging. Logs are stored to the service's app persistent storage. The service creates loggers, enumerates existing logs and creates log readers for apps that want to consume logs. Currently the service will only enumerate logs that existed when the service started. There's no support for reading logs that are currently open. I also need to add the ability to log generic text/file/line entries. R=kulakowski@chromium.org Committed: https://chromium.googlesource.com/external/mojo/+/f6adcd2ac60af3897b5d9b2e05dfd6a7777da11d

Patch Set 1 #

Total comments: 1

Patch Set 2 : Fixes per feedback. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+937 lines, -116 lines) Patch
M mojo/dart/packages/mojo_services/lib/mojo/flog/flog.mojom.dart View 1 19 chunks +111 lines, -102 lines 0 comments Download
M mojo/services/flog/cpp/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M mojo/services/flog/cpp/flog.h View 2 chunks +4 lines, -2 lines 0 comments Download
M mojo/services/flog/interfaces/flog.mojom View 5 chunks +13 lines, -9 lines 0 comments Download
M services/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
A services/flog/BUILD.gn View 1 chunk +29 lines, -0 lines 0 comments Download
A services/flog/flog_directory.h View 1 1 chunk +52 lines, -0 lines 0 comments Download
A services/flog/flog_directory.cc View 1 1 chunk +111 lines, -0 lines 0 comments Download
A services/flog/flog_logger_impl.h View 1 chunk +53 lines, -0 lines 0 comments Download
A services/flog/flog_logger_impl.cc View 1 chunk +74 lines, -0 lines 0 comments Download
A services/flog/flog_reader_impl.h View 1 chunk +86 lines, -0 lines 0 comments Download
A services/flog/flog_reader_impl.cc View 1 chunk +254 lines, -0 lines 0 comments Download
A services/flog/flog_service_impl.h View 1 1 chunk +52 lines, -0 lines 0 comments Download
A services/flog/flog_service_impl.cc View 1 1 chunk +93 lines, -0 lines 0 comments Download
A + services/flog/main.cc View 1 chunk +3 lines, -3 lines 0 comments Download

Messages

Total messages: 8 (3 generated)
dalesat
Please take a look. Thanks!
4 years, 6 months ago (2016-06-06 19:06:30 UTC) #3
kulakowski
Otherwise seems ok. Still reviewing your doc. https://codereview.chromium.org/2046703002/diff/1/services/flog/flog_directory.h File services/flog/flog_directory.h (right): https://codereview.chromium.org/2046703002/diff/1/services/flog/flog_directory.h#newcode22 services/flog/flog_directory.h:22: std::function<void(std::map<uint32_t, std::string>&&)>; ...
4 years, 6 months ago (2016-06-13 20:14:26 UTC) #4
dalesat
PTAL
4 years, 6 months ago (2016-06-14 16:49:57 UTC) #5
kulakowski
lgtm
4 years, 6 months ago (2016-06-15 22:10:18 UTC) #6
dalesat
4 years, 6 months ago (2016-06-15 22:41:39 UTC) #8
Message was sent while issue was closed.
Committed patchset #2 (id:20001) manually as
f6adcd2ac60af3897b5d9b2e05dfd6a7777da11d (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698