Closed Bug 1242091 Opened 6 years ago Closed 9 months ago

[lint: LongLogTag] Decide how to handle long log tags (perhaps unify logging style)

Categories

(Firefox for Android Graveyard :: General, defect)

All
Android
defect
Not set
normal

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)
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)
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)
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.
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.
I'm not going to get to this anytime soon.
Assignee: michael.l.comella → nobody
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.
(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
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: 9 months ago
Resolution: --- → INCOMPLETE
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.