[dart][logger] include logger name in tags
TEST=
- fx run-host-tests fuchsia_logger_package_unittests
Change-Id: Ia3c9374ef1947e016005158038f0786835fbdfc0
diff --git a/public/dart/fuchsia_logger/lib/src/internal/_fuchsia_log_writer.dart b/public/dart/fuchsia_logger/lib/src/internal/_fuchsia_log_writer.dart
index 35dff59..d3c94ce 100644
--- a/public/dart/fuchsia_logger/lib/src/internal/_fuchsia_log_writer.dart
+++ b/public/dart/fuchsia_logger/lib/src/internal/_fuchsia_log_writer.dart
@@ -2,6 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+import 'dart:async';
+
import 'package:logging/logging.dart';
import 'package:meta/meta.dart';
import 'package:fidl_fuchsia_logger/fidl_async.dart' as fidl_logger;
@@ -19,10 +21,10 @@
zircon.Socket _socket;
/// Constructor
- FuchsiaLogWriter({@required Logger logger})
- : assert(logger != null),
+ FuchsiaLogWriter({@required Stream<LogRecord> logStream})
+ : assert(logStream != null),
super(
- logger: logger,
+ logStream: logStream,
shouldBufferLogs: true,
) {
_connectToSysLogger();
diff --git a/public/dart/fuchsia_logger/lib/src/internal/_log_message.dart b/public/dart/fuchsia_logger/lib/src/internal/_log_message.dart
index 92238f3..24b835a 100644
--- a/public/dart/fuchsia_logger/lib/src/internal/_log_message.dart
+++ b/public/dart/fuchsia_logger/lib/src/internal/_log_message.dart
@@ -34,7 +34,18 @@
/// The initial log record
final LogRecord record;
- /// Any additional tags to append to the record.
+ /// The base name for the logger. This is the name that is assigned in
+ /// setupLogger whereas the name in the record is the name that comes from
+ /// the dart logger instance.
+ final String loggerBaseName;
+
+ /// A string that can be included to identify the call site of this log message
+ final String codeLocation;
+
+ /// Any additional tags to append to the record. When the record it sent to the
+ /// logger it will include these tags after the name and code location if they are
+ /// present. Any tags which are over the limit of the [_maxCombinedTags] will
+ /// be dropped.
final List<String> tags;
/// The id of the process which this log message is associated with
@@ -53,6 +64,8 @@
@required this.record,
@required this.processId,
@required this.threadId,
+ this.loggerBaseName,
+ this.codeLocation,
this.tags = const [],
}) : assert(record != null),
assert(processId != null),
@@ -69,15 +82,22 @@
..setUint32(28, 0, Endian.little); // TODO(120860552) droppedLogs
int byteOffset = 32;
- // Write global tags
int totalTagCount = 0;
- for (final tag in tags) {
- if (tag != null && totalTagCount < _maxCombinedTags) {
+ void addTag(String tag) {
+ if (totalTagCount < _maxCombinedTags) {
byteOffset = _setTag(bytes, byteOffset, tag);
totalTagCount++;
}
}
+ addTag(loggerBaseName);
+ addTag(record.loggerName);
+ addTag(codeLocation);
+
+ if (tags != null) {
+ tags.forEach(addTag);
+ }
+
// We need to skip the local tags section since we do not support them
bytes.setUint8(byteOffset++, 0);
@@ -129,7 +149,7 @@
}
int _setTag(ByteData bytes, int byteOffset, String tag) {
- if (tag == null || tag == 'null') {
+ if (tag == null || tag == 'null' || tag.isEmpty) {
return byteOffset;
}
int nextOffset = _setString(bytes, byteOffset + 1, tag, _maxTagLength);
diff --git a/public/dart/fuchsia_logger/lib/src/internal/_log_writer.dart b/public/dart/fuchsia_logger/lib/src/internal/_log_writer.dart
index 2deab8f..4055a13 100644
--- a/public/dart/fuchsia_logger/lib/src/internal/_log_writer.dart
+++ b/public/dart/fuchsia_logger/lib/src/internal/_log_writer.dart
@@ -24,6 +24,9 @@
StreamController<LogMessage> _controller;
+ /// A name which will prefix each log message and can be used for filtering.
+ String loggerBaseName;
+
/// If set to true, this method will include the stack trace
/// in each log record so we can later extract out the call site.
/// This is a heavy operation and should be used with caution.
@@ -31,9 +34,9 @@
/// Constructor
LogWriter({
- @required Logger logger,
+ @required Stream<LogRecord> logStream,
bool shouldBufferLogs = false,
- }) : assert(logger != null) {
+ }) : assert(logStream != null) {
void Function(LogMessage) onMessageFunc;
if (shouldBufferLogs) {
@@ -46,8 +49,7 @@
} else {
onMessageFunc = onMessage;
}
- logger.onRecord.listen(
- (record) => onMessageFunc(_messageFromRecord(record)),
+ logStream.listen((record) => onMessageFunc(_messageFromRecord(record)),
onDone: () => _controller?.close());
}
@@ -79,7 +81,9 @@
record: record,
processId: pid,
threadId: Isolate.current.hashCode,
- tags: _tagsForLogMessage(),
+ loggerBaseName: loggerBaseName,
+ codeLocation: _codeLocation(),
+ tags: _globalTags,
);
/// A method for subclasses to implement to handle messages as they are
@@ -95,9 +99,9 @@
List<String> _verifyGlobalTags(List<String> tags) {
List<String> result = <String>[];
- // make our own copy to allow us to remove null values an not change the
+ // make our own copy to allow us to remove null values and not change the
// original values
- final incomingTags = List.of(tags)
+ final incomingTags = List.of(tags ?? const [])
..removeWhere((t) => t == null || t.isEmpty);
if (incomingTags != null) {
@@ -119,14 +123,11 @@
return result;
}
- List<String> _tagsForLogMessage() {
+ String _codeLocation() {
if (forceShowCodeLocation) {
- final codeLocation = _codeLocationFromStackTrace(StackTrace.current);
- if (codeLocation != null && codeLocation.isNotEmpty) {
- return List.of(_globalTags)..add(codeLocation);
- }
+ return _codeLocationFromStackTrace(StackTrace.current);
}
- return _globalTags;
+ return null;
}
String _codeLocationFromStackTrace(StackTrace stackTrace) {
diff --git a/public/dart/fuchsia_logger/lib/src/internal/_stdout_log_writer.dart b/public/dart/fuchsia_logger/lib/src/internal/_stdout_log_writer.dart
index b5141d0..c6e2a9d 100644
--- a/public/dart/fuchsia_logger/lib/src/internal/_stdout_log_writer.dart
+++ b/public/dart/fuchsia_logger/lib/src/internal/_stdout_log_writer.dart
@@ -2,6 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+import 'dart:async';
+
import 'package:logging/logging.dart';
import 'package:meta/meta.dart';
@@ -11,10 +13,10 @@
/// A concrete implementation of [LogWriter] which prints the logs to stdout.
class StdoutLogWriter extends LogWriter {
/// Constructor
- StdoutLogWriter({@required Logger logger})
- : assert(logger != null),
+ StdoutLogWriter({@required Stream<LogRecord> logStream})
+ : assert(logStream != null),
super(
- logger: logger,
+ logStream: logStream,
shouldBufferLogs: false,
);
@@ -24,6 +26,16 @@
getLevelString(message.record.level),
];
+ void addToScopes(String scope) {
+ if (scope != null && scope.isNotEmpty) {
+ scopes.add(scope);
+ }
+ }
+
+ addToScopes(message.loggerBaseName);
+ addToScopes(message.record.loggerName);
+ addToScopes(message.codeLocation);
+
message.tags.forEach(scopes.add);
String scopesString = scopes.join(':');
diff --git a/public/dart/fuchsia_logger/lib/src/logger/logger.dart b/public/dart/fuchsia_logger/lib/src/logger/logger.dart
index cea6821..dbf3929 100644
--- a/public/dart/fuchsia_logger/lib/src/logger/logger.dart
+++ b/public/dart/fuchsia_logger/lib/src/logger/logger.dart
@@ -47,10 +47,11 @@
/// [Level.ALL].
///
/// If [globalTags] is provided, these tags will be added to each message logged
-/// via this logger. The logger can accept 5 global tags, however, one of those
-/// tags is reserved for the logger name and one is reserved for the code
-/// location if it is requested. The logger will drop any tags that are over
-/// this limit.
+/// via this logger. The system logger can only accept 5 tags with each log
+/// record. Each record will include the name provided in the [setupLogger]
+/// method, the name of the dart logger if it is not the root logger and the
+/// code location if it is requested. Any tags that are over the maximum limit
+/// that the system is allowed to receive will be dropped.
///
/// By default, the caller code location is automatically added in checked mode
/// and not in production mode, because it is relatively expensive to calculate
@@ -71,16 +72,13 @@
// method will be a noop. At this point, _logWriter will not be null
_connectToLogWriterIfNeeded();
- // We need to provide a name for the logger in the form of a tag because
- // we are not using hierarchical logging.
+ final loggerBaseName = name ??
+ Platform.script?.pathSegments?.lastWhere((_) => true, orElse: () => null);
+
// Tags get appended to each log statement. We put the name, if present
// as the first tag so it makes it easier to identify.
// We remove any null values before sending them to the logger
- final List<String> tags = [
- name ??
- Platform.script?.pathSegments
- ?.lastWhere((_) => true, orElse: () => null)
- ]
+ final List<String> tags = []
..addAll(globalTags ?? const [])
..removeWhere((t) => t == null || t.isEmpty);
@@ -91,6 +89,7 @@
}());
_logWriter
+ ..loggerBaseName = loggerBaseName
..globalTags = tags
..forceShowCodeLocation = forceShowCodeLocation ?? inCheckedMode;
}
@@ -104,9 +103,11 @@
return;
}
+ // we do not use a named logger here because we want to make sure that
+ // the provided name is included as a separate tag.
if (Platform.isFuchsia) {
- _logWriter = FuchsiaLogWriter(logger: Logger.root);
+ _logWriter = FuchsiaLogWriter(logStream: Logger.root.onRecord);
} else {
- _logWriter = StdoutLogWriter(logger: Logger.root);
+ _logWriter = StdoutLogWriter(logStream: Logger.root.onRecord);
}
}
diff --git a/public/dart/fuchsia_logger/test/internal/log_message_test.dart b/public/dart/fuchsia_logger/test/internal/log_message_test.dart
index 627afb9..6497acd 100644
--- a/public/dart/fuchsia_logger/test/internal/log_message_test.dart
+++ b/public/dart/fuchsia_logger/test/internal/log_message_test.dart
@@ -29,6 +29,7 @@
_validateFixedBlock(buffer, 0, 1, 2);
expect(buffer[32], equals(4));
+ // The name of the logger
expect(utf8.decode(buffer.sublist(33, 37)), equals('TEST'));
expect(utf8.decode(buffer.sublist(38, 41)), equals('foo'));
expect(buffer[41], equals(0));
@@ -44,6 +45,7 @@
final buffer = bytes.buffer.asUint8List();
_validateFixedBlock(buffer, 3, 1, 2);
+ // The name of the logger
expect(buffer[32], equals(4));
expect(utf8.decode(buffer.sublist(33, 37)), equals('TEST'));
int end = 37;
@@ -73,6 +75,7 @@
final buffer = bytes.buffer.asUint8List();
_validateFixedBlock(buffer, 2, 1, 2);
+ // The name of the logger
expect(buffer[32], equals(4));
expect(utf8.decode(buffer.sublist(33, 37)), equals('TEST'));
int end = 37;
@@ -104,6 +107,7 @@
final buffer = bytes.buffer.asUint8List();
_validateFixedBlock(buffer, -2, 1, 2);
+ // The name of the logger
expect(buffer[32], equals(4));
expect(utf8.decode(buffer.sublist(33, 37)), equals('TEST'));
int start = 37;
@@ -130,12 +134,50 @@
expect(buffer[end++], equals(0));
expect(bytes.lengthInBytes, equals(end));
});
+
+ test('convert to bytes with tags and named Logger', () {
+ final tags = ['tag'];
+ final message = _makeMessage(
+ Level.FINE,
+ 'bar',
+ tags: tags,
+ loggerName: 'test-logger',
+ );
+
+ final bytes = message.toBytes();
+ final buffer = bytes.buffer.asUint8List();
+ _validateFixedBlock(buffer, -2, 1, 2);
+
+ // The name of the logger
+ expect(buffer[32], equals(4));
+ expect(utf8.decode(buffer.sublist(33, 37)), equals('TEST'));
+ int start = 37;
+
+ // verify that the named logger gets added after the tags
+ expect(buffer[start], equals('test-logger'.length));
+ int end = start + buffer[start] + 1;
+ start++;
+ expect(utf8.decode(buffer.sublist(start, end)), equals('test-logger'));
+
+ // verify the first tag
+ start = end;
+ expect(buffer[start], equals(tags[0].length));
+ end = start + buffer[start] + 1;
+ start++;
+ expect(utf8.decode(buffer.sublist(start, end)), equals(tags[0]));
+
+ // dividing 0 byte
+ expect(buffer[end++], equals(0));
+
+ start = end;
+ expect(utf8.decode(buffer.sublist(start, start + 3)), equals('bar'));
+ end = start + 3;
+ expect(buffer[end++], equals(0));
+ expect(bytes.lengthInBytes, equals(end));
+ });
});
}
-List<String> _allTags(String name, List<String> tags) =>
- (name != null ? [name] : [])..addAll(tags ?? []);
-
/// Convert from little endian format bytes to an unsiged 32 bit int.
int _bytesToInt32(List<int> bytes) {
ByteData byteData = ByteData(4);
@@ -160,12 +202,14 @@
String name = 'TEST',
Object error,
StackTrace stackTrace,
- List<String> tags}) =>
+ List<String> tags,
+ String loggerName = ''}) =>
LogMessage(
- record: LogRecord(level, message, null, error, stackTrace),
+ record: LogRecord(level, message, loggerName, error, stackTrace),
processId: processId,
threadId: threadId,
- tags: _allTags(name, tags),
+ loggerBaseName: name,
+ tags: tags,
);
void _validateFixedBlock(
diff --git a/public/dart/fuchsia_logger/test/internal/log_writer_test.dart b/public/dart/fuchsia_logger/test/internal/log_writer_test.dart
index 542b4c0..ce040dd 100644
--- a/public/dart/fuchsia_logger/test/internal/log_writer_test.dart
+++ b/public/dart/fuchsia_logger/test/internal/log_writer_test.dart
@@ -18,7 +18,7 @@
Logger logger;
setUp(() {
- logger = Logger('foo');
+ logger = Logger.root;
});
tearDown(() {
@@ -128,18 +128,18 @@
});
test('setting forceShowCodeLocation includes code location in tags', () {
- List<String> tags;
+ String codeLocation;
StubLogWriter(
logger: logger,
onMessageFunc: (m) {
- tags = m.tags;
+ codeLocation = m.codeLocation;
}).forceShowCodeLocation = true;
logger.info('foo');
- expect(tags[0], matches(r'log_writer_test.dart\(\d+\)'));
+ expect(codeLocation, matches(r'log_writer_test.dart\(\d+\)'));
});
- }, timeout: Timeout(Duration(milliseconds: 100)));
+ });
}
class StubLogWriter extends LogWriter {
@@ -149,7 +149,7 @@
this.onMessageFunc,
Logger logger,
shouldBufferLogs = false,
- }) : super(logger: logger, shouldBufferLogs: shouldBufferLogs);
+ }) : super(logStream: logger.onRecord, shouldBufferLogs: shouldBufferLogs);
@override
void onMessage(LogMessage message) => this.onMessageFunc(message);
diff --git a/public/dart/fuchsia_logger/test/internal/stdout_log_writer_test.dart b/public/dart/fuchsia_logger/test/internal/stdout_log_writer_test.dart
index 6e957dd..60a91b1 100644
--- a/public/dart/fuchsia_logger/test/internal/stdout_log_writer_test.dart
+++ b/public/dart/fuchsia_logger/test/internal/stdout_log_writer_test.dart
@@ -74,6 +74,13 @@
logger.info('foo');
}, (l) => expect(l, matches(r'INFO:stdout_log_writer_test.dart\(\d+\)')));
});
+
+ test('includes named logger name', () {
+ _overridePrint((logger, writer) {
+ logger.info('foo');
+ }, (l) => expect(l, '[INFO:named-logger] foo'),
+ namedLogger: Logger('named-logger'));
+ });
});
group('tags', () {
@@ -89,12 +96,15 @@
// helper method for capturing stdout. Code executed within the [zoned] function
// will pass the logged output to the [receiver] function with the line that
// would have gone to stdout.
-void _overridePrint(void Function(Logger, StdoutLogWriter) zoned,
- void Function(String) receiver) {
+void _overridePrint(
+ void Function(Logger, StdoutLogWriter) zoned,
+ void Function(String) receiver, {
+ Logger namedLogger,
+}) {
runZoned(
() {
- final logger = Logger('foo');
- zoned(logger, StdoutLogWriter(logger: logger));
+ final logger = namedLogger ?? Logger.root;
+ zoned(logger, StdoutLogWriter(logStream: logger.onRecord));
logger.clearListeners();
},
zoneSpecification: ZoneSpecification(
