botframework-sdk icon indicating copy to clipboard operation
botframework-sdk copied to clipboard

Send Trace Activities only if environment is "development"

Open stevengum opened this issue 5 years ago • 3 comments
trafficstars

This request iterates on https://github.com/microsoft/botbuilder-dotnet/issues/4645

Issue

There is a lot of trace activities being sent over the wire, when it's not required. Instead, trace activity generation should be based off of the bot's environment.

Proposed change

Have TurnContext not send trace activities depending on the bot's environment.

  • If the bot is in a production or staging environment, do not send trace activities.
  • If the bot is in a development environment, do send trace activities.

The implementation for this should vary across runtimes/languages as each runtime/language has a different idiomatic way of detecting the application's environment.

Tracking Status

Dotnet SDK TODO

  • [ ] PR
  • [ ] Merged

Javascript SDK TODO

  • [ ] PR
  • [ ] Merged

Python SDK TODO

  • [ ] PR
  • [ ] Merged

Java SDK TODO

  • [ ] PR
  • [ ] Merged

Samples TODO

  • [ ] PR
  • [ ] Merged

Docs TODO

  • [ ] PR
  • [ ] Merged

Tools TODO

  • [ ] PR
  • [ ] Merged

stevengum avatar Oct 21 '20 16:10 stevengum

One drawback is that Azure apparently doesn't set NODE_ENV when you specify Node.js as the stack runtime on code-approach Web Apps. However, using NODE_ENV is the norm for Node developers, so I believe we should use this anyways.

That said, we should update the ARM templates to set the NODE_ENV to production if it's true that Azure doesn't set it.


Edit: I can confirm that NODE_ENV isn't set in the Configuration pane on Azure for Linux or Windows App Services, but I'm cautiously hopeful that it's still being set somewhere else...

stevengum avatar Oct 21 '20 16:10 stevengum

There are cases where tracing can be useful in production. (sidecar debugging a specific conversation)

EricDahlvang avatar Oct 21 '20 17:10 EricDahlvang

That sounds reasonable and could be easily addressed by using a BF-specific environment variable that defaults to the language-specific environment variable.

const { SIDECAR_TRACING, NODE_ENV } = process.env;

// Rough draft at logic
if (SIDECAR_TRACING || NODE_ENV === 'development') {
    // send sidecar traces
}

The initial ask's solution should be investigated more and we should probably reach out to partner teams to make sure we don't step on any toes.

stevengum avatar Oct 21 '20 17:10 stevengum