mobile: Pass the log level when setting a logger

[fredyw]

This PR adds a feature to allow the underlying logging implementation log Envoy logs according to the log levels when setting the logger via setLogger.

For example (for Android):

engineBuilder
  .setLogger { level, msg ->
    if (level == TRACE) {
      Log.v(tag, msg)
    } else if (level == DEBUG) {
      Log.d(tag, msg)
    } else if (level == INFO) {
      Log.i(tag, msg)
    } else if (level == WARN) {
      Log.w(tag, msg)
    } else if (level == ERROR) {
      Log.e(tag, msg)
    } else if (level == CRITICAL) {
      Log.wtf(tag, msg)
    }
  }

Risk Level: low
Testing: unit tests
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: mobile

[repokitteh-read-only]

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #32131 was opened by fredyw.

see: more, trace.

[fredyw]

/assign @abeyad

[abeyad]

Nice!

[abeyad]

I know LambdaDelegate is what it's been called from before, but do you know why it's called that? I don't understand what it means. It seems like it has an envoy_logger, which differs it from the DefaultDelegate

[fredyw]

My guess is because it takes a function pointer, hence lambda. I agree that it's probably not a very good name. LoggerDelegate (to replace LambdaDelegate) and StderrLoggerDelagate (to replace DefaultDelegate) will probably better names.

[abeyad]

I'm guessing the int is meant to represent the log level? What does the NSString* represent? Should we make this a struct that can also be used in Objective-C? Also, I thought we'd need to use a NSNumber here

[fredyw]

The NSString* represents the message. I think using a struct is probably overkill given that we only have 2 parameters. Plus, this structure is similar to the C++ function pointer definition. As for using NSNumber, I think using NSInteger is better given that LogLevel is an Int:

envoy/mobile/library/swift/LogLevel.swift

Line 6 in bb80b32

public enum LogLevel: Int {

So, I updated the code to use NSInteger instead of int.

[abeyad]

Do we need any tets for non-INFO level (I assume that's the default), e.g. log level set to DEBUG and make sure we get debug but not trace messages?

[fredyw]

The current CronvoyLogger implementation doesn't actually care about the log level and it doesn't even delegate the logging to the underlying logging implementation. It simply logs the messages to disk. I have a plan to fix this in Cronvoy similar to how we can configure the logger with direct EngineBuilder.

The tests that do log + level are in C++. See LambdaDelegateWithLevelTest.

[fredyw]

/retest

[fredyw]

/retest