Closed
Bug 1242091
Opened 9 years ago
Closed 4 years ago
[lint: LongLogTag] Decide how to handle long log tags (perhaps unify logging style)
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
INCOMPLETE
People
(Reporter: mcomella, Unassigned)
References
Details
From the warning:
Log tags are only allowed to be at most 23 tag characters long.
Presumably, the system doesn't display long log tags but it's unclear what it does instead, whether truncating the tags or something else.
Do we want to truncate them? Come up with custom log tags rather than ones using the class names?
Additionally, should we maintain the current log style ("Gecko" + className)? Wouldn't "Fennec" + className be more correct?
If we decide to truncate and still use the class names, I'd vote we have a static method somewhere to handle crafting these log tags.
Nick, Richard: do you have any opinions?
Flags: needinfo?(rnewman)
Flags: needinfo?(nalexander)
Comment 1•9 years ago
|
||
From reading the Android source, assuming nothing changed recently, long tags are only a problem if you check log levels. Otherwise the length is quietly ignored. It's still wrong, but one normally doesn't suffer any consequences.
The right answer is perhaps one of these two:
* Manually truncate. I catch these in review.
* Switch to using Sync's much more civilized Logger class, which has a per-thread log tag and then per-caller log tags with no length limit, as well as more efficient log level checking and a bunch of other happiness.
Re naming: "Gecko" is as arbitrary as anything else, I suppose.
Flags: needinfo?(rnewman)
Comment 2•9 years ago
|
||
I see no reason to churn Gecko to something else. It used to be that Android complained or crashed with long tags, but that's no longer the case. We should still catch them and reduce them manually.
I'd be pleased to use Logger throughout, and eventually transition to a more industrial-strength solution like https://github.com/JakeWharton/timber.
Flags: needinfo?(nalexander)
Comment 3•9 years ago
|
||
I remember seeing weird things happening with very long tags (In a self-written log class I automatically created file name + line number tags) like the log message being logged with a random different (system) tag: This is super confusing and you get crazy searching for your log message in a filtered log.
Reporter | ||
Comment 4•9 years ago
|
||
In bug 1205835, I did this truncation by hand in TelemetryUploadService but if we decide to outsource to a utility method, we should fix this case.
Reporter | ||
Comment 5•9 years ago
|
||
I'm not going to get to this anytime soon.
Assignee: michael.l.comella → nobody
Comment 6•9 years ago
|
||
Having a quick look at the code we have a mix of:
"CustomStringThat'sUsuallyTheClassName"
"Gecko" + XXX.class.getSimpleName();
StringUtils.safeSubstring("Gecko" + XXX.class.getSimpleName(), 0, 23);
AppGlobals.makeLogTag(XXX.class.getSimpleName());
I feel like it's probably worth consistently using AppGlobals.makeLogTag() everywhere so that we track the 23 character limitation in one place, regardless of whether the decision is to use the "Gecko" prefix consistently. We could even just pass the class directly into makeLogTag, and do classname extraction (and any further manipulations) there.
Reporter | ||
Comment 7•9 years ago
|
||
(In reply to Andrzej Hunt :ahunt from comment #6)
> I feel like it's probably worth consistently using AppGlobals.makeLogTag()
> everywhere
The general idea of using a single function to funnel this all through, I agree with. However:
* AppGlobals is in stumbler. In IJ, the dependencies are fine to access this code but it's unclear if the same will happen with moz.build
* It uses its own _PREFIX constant that's stumbler specific
Comment 8•4 years ago
|
||
We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INCOMPLETE
Assignee | ||
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•