Introduce LegacyAppender that forwards log records to the legacy platform.

[azasypkin]

LegacyAppender just forwards all log records it receives to the kbnServer.server.log so that legacy platform can decide how to format that records and where it should be written to.

From the new-platform perspective LegacyAppender is set as default appender used by the root logger.

Fixes #13412.

[azasypkin]

Hey @kjbekkelund,

Here is implementation of the 2nd option I mentioned in the #13412. You can also take a look at the first commit that is what I meant by option 1 :)

[azasypkin]

note: it seems in the legacy platform we can't provide both message and error :/

[azasypkin]

Env is now everywhere :)

[kimjoar]

We now export interface LegacyKbnServer, but I wonder if we should make that a class, and construct it with the kbnServer as input. The goal would be to never pass the raw kbnServer around, so we could just have it be any within the LegacyKbnServer class. However, the LegacyKbnServer class would expose the specific apis we want to open up in the new platform, e.g. for logging etc. Does that make sense? You've touched way more than me on this code, so their might be nuances I'm not seeing.

[kimjoar]

(I know this isn't too far away from what we're doing, as we're specifying the interface, I'm just trying to make it even stricter — just not sure if it makes sense 🙈)

[kimjoar]

Looks like it works as expected. LGTM.

Just that one question re legacyKbnServer. If you want to we could also do that in a follow-up PR, so I'm good with merging this as-is.

[kimjoar]

Great use of snapshots imo 🎉

[azasypkin]

Yeah, it works pretty well for cases when arguments are predictable, borrowed the approach from your logger mock's _collect method :)

[azasypkin]

We now export interface LegacyKbnServer, but I wonder if we should make that a class, and construct it with the kbnServer as input. The goal would be to never pass the raw kbnServer around, so we could just have it be any within the LegacyKbnServer class. However, the LegacyKbnServer class would expose the specific apis we want to open up in the new platform, e.g. for logging etc. Does that make sense? You've touched way more than me on this code, so their might be nuances I'm not seeing.

Do you mean something like this?

export class LegacyKbnServer {
  constructor(private readonly rawKbnServer: any) {
  }

  get newPlatformProxyListener() {
    return this.rawKbnServer.newPlatformProxyListener;
  }

  log(...args: any[]) {
    this.rawKbnServer.server.log(...args);
  }
}
---------------
export const injectIntoKbnServer = (rawKbnServer: any) => {
  const legacyConfig$ = new BehaviorSubject(rawKbnServer.config);
  const config$ = legacyConfig$.map(
    legacyConfig => new LegacyConfigToRawConfigAdapter(legacyConfig)
  );

  rawKbnServer.updateNewPlatformConfig = (legacyConfig: LegacyConfig) => {
    legacyConfig$.next(legacyConfig);
  };

  //const LegacyPlatformProxifier =
  rawKbnServer.newPlatformProxyListener = new LegacyPlatformProxifier(
    new Root(config$, Env.createDefault({ kbnServer: new LegacyKbnServer(rawKbnServer) }))
  );
};

[kimjoar]

Yep

[azasypkin]

@kjbekkelund Thanks for review. Would you mind taking a look at the 4th commit with LegacyKbnServer class?

[kimjoar]

I like it. LGTM.

[kimjoar]

nitpick: it -> test?

[kimjoar]

(I'm not sure here, as I don't know if we are consistent either way, so feel free to skip)

[azasypkin]

ha-ha, right, it should definitely be test - I believe we are pretty consistent on this in the new platform code (I see only 3 more tests that use it and that was me who did that :), so I'll change them to test too).