Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow TelemetryInitializer properties to be overridden by user #1406

Merged
merged 1 commit into from
Apr 3, 2019

Conversation

daveta
Copy link
Contributor

@daveta daveta commented Feb 27, 2019

Fix issue that VA encountered.
Allow users to override telemetry that's logged from the initializer.

@coveralls
Copy link
Collaborator

Pull Request Test Coverage Report for Build 50132

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.04%) to 74.599%

Totals Coverage Status
Change from base Build 49945: 0.04%
Covered Lines: 4003
Relevant Lines: 5366

💛 - Coveralls

@cleemullins
Copy link
Contributor

@daveta Does this need to be updated on the JS side as well?

@cleemullins cleemullins added blocked Current progress is blocked on something else. Parity with JS Required labels Feb 28, 2019
@daveta
Copy link
Contributor Author

daveta commented Mar 19, 2019

Small update here:

  • I've got an outstanding query with Ben about functionality of nodejs.
  • I believe nodejs at the moment has a bug where it's not placing the bot properties (so no chance of collision!)
  • Based on implementation in nodejs, no chance of user overrides throwing an exception (like C#).
  • HOWEVER, depending on the nodejs Application Insights hook we are using, it may not be possible to override the bot properties. It all boils down to the order in which the hook gets called. So, until (a) the bug gets fixed and (b) we can understand the order of invocation in nodejs, let's sit on this for a bit. If a user cannot override, then for parity, we shouldn't "fix" this bug..

@cleemullins
Copy link
Contributor

@daveta Any update on this?

@daveta
Copy link
Contributor Author

daveta commented Apr 2, 2019

Ok, so it appears the hook that we initialize the bot properties (now fixed) should be be overridable given restify calls it first.

It should behave the same way on both C# and node.

@daveta daveta removed the blocked Current progress is blocked on something else. label Apr 2, 2019
@cleemullins
Copy link
Contributor

Per @daveta, "Yes, in js it's already there - you can consider this a js equivalence".

Merging.

@cleemullins cleemullins merged commit 58b22fa into master Apr 3, 2019
@cleemullins cleemullins deleted the daveta-fixAiException branch April 3, 2019 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants