Allow Local environment detection (or have Local be the default environment)

[mtwilliams5]

We're using this library with our lambda function for creating embedded metrics, and it works fantastically for that use case, but we've been struggling a little bit with getting it all to play smoothly during local development. I'm aware that we can set AWS_EMF_ENVIRONMENT to Local at runtime to make the library use stdout, and we've added that to a few of our package.json scripts to make it easier for everyone, but there are still a few cases where there's no simple or easy route to setting this, or somebody does something a little different and forgets to add that environment variable (we work in a large monorepo project, so the number of people contributing who know many detail around EMF is fairly low). This leads to confusing error messages about UnhandledPromiseRejectionWarning: Error: connect ECONNREFUSED 0.0.0.0:25888 when running in Node 12 - and for anyone who is running in Node 14, it seems to actually just crash the local server.

Looking into the library, it looks like Agent is set up as the default environment, and the Local environment probe method is intentionally a no-op, which means that the only way to use the library in local environment mode is via this environment variable.

Is there a historical reason why it is this way? Is there no other way to detect if the environment should be Agent, and thus switch the default environment around, or having some other way to auto-detect if the environment should be Local (e.g. if NODE_ENV !== 'production')?

[jaredcnance]

Agent is the default because most production systems will require an agent and we wanted it to work out-of-the-box so to speak. The change you’re describing is a breaking change and I’m not convinced yet that this would generally be the preferred behavior. Can you not set the environment var in your package.json? Something like:

"scripts": {
  "local": "AWS_EMF_ENVIRONMENT=Local node app.js",
  "start": "node app.js"
}

Alternatively, we could defer environment detection until after modules are loaded, perhaps on the first logger flush. This would solve #43 and could allow us to let you choose how environment selection works in your app. This would cause increased flush delays for the first flush in some environments, EC2 and ECS, but I think those environments can likely tolerate this delay. Also, if we implement #36 it may have very little impact.

[mtwilliams5]

We can (and have) set it in the package.json, but due to the complexity of our project (it's a fairly large monorepo), we end up having to have that value in a fair few different places - there's the script to start the dev server, then our browser tests also start a server to run, so it needs to go in there too, then we have lighthouse tests that do the same thing, and so on. It ends up leading to that value having to be put in multiple places, or relying on everyone working in the repo adding the environment variable to their shell profile scripts.

It's also generated a fair few questions, since we added it in, from people not familiar with the infrastructure side of the project asking 'what is this AWS EMF environment thing? What is it used for? Do I need to do anything different with it if I wanted to test my work with test data from our upstream?'. It's a simple enough question for us to answer, but it's been asked enough times by enough different people for me to be wanting to look for a way of not needing to have it there to avoid the confusion.

[jaredcnance]

Thanks for the details. I think the proper solution to this is the one I described previously. We need to be able to give you more control around how environment detection works. In the meantime, I think something like this could work for you:

const resolveEnvironment = async (): Promise<IEnvironment> => {
  // some custom logic around how you want to determine the environment
  return isLocal 
    ? new LocalEnvironment() 
    : //...;
};

const createLoggerForMyEnvironment = (): MetricsLogger => {
  const context = MetricsContext.empty();
  const logger = new MetricsLogger(resolveEnvironment, context);
  return logger;
};

[eyalroth]

@jaredcnance I believe this won't work as LocalEnvironment or any of the environment-related definitions are not exported by the library.

[MattCopenhaver]

I too would be an advocate of the default behavior being Local, as it makes no assumptions about your setup.

most production systems will require an agent and we wanted it to work out-of-the-box so to speak. I question whether this is necessarily true, given Lambda does not require an agent. And, even production system that do require an agent will also need their local development environment to be configured without one.

It's much easier to set an environment variable in your production or pre-production AWS environments, than to make every developer aware of this quirk in their local setup. Adding it to your package.json scripts definitely helps, but is not foolproof.

I don't think this would be much of a problem if it were possible to configure this variable in the code, though.

[MattCopenhaver]

I'm getting an error message trying to edit my last comment, but it should read:

most production systems will require an agent and we wanted it to work out-of-the-box so to speak.

I question whether this is necessarily true, given Lambda does not require an agent. And, even production system that do require an agent will also need their local development environment to be configured without one.

[jensbodal]

@jaredcnance I believe this won't work as LocalEnvironment or any of the environment-related definitions are not exported by the library.

They're not exported at the root module but you can access them

import { Configuration, MetricsLogger, Unit } from 'aws-embedded-metrics';
import { IEnvironment } from 'aws-embedded-metrics/lib/environment/IEnvironment';
import { LocalEnvironment } from 'aws-embedded-metrics/lib/environment/LocalEnvironment';
import { MetricsContext } from 'aws-embedded-metrics/lib/logger/MetricsContext';

@Injectable()
export class MetricsLoggerService {
  private metricsLogger: MetricsLogger;

  constructor() {
    const context = MetricsContext.empty();
    this.metricsLogger = new MetricsLogger(this.resolveEnvironment, context);
    Configuration.namespace = 'Foo';

    this.metricsLogger.setDimensions();
  }

  private async resolveEnvironment(): Promise<IEnvironment> {
    return new LocalEnvironment();
  }
}

I haven't tested that I can change environments after bootstrapping my nestJs app but I have tested that the local environment works this way without needing to set the environment variable.

It would be nice if this was a little cleaner to setup.