Skip to content

Conversation

@norvin-yecla-lrn
Copy link
Contributor

@norvin-yecla-lrn norvin-yecla-lrn commented Jul 29, 2019

This PR adds telemetry to all requests made using the ASP.NET SDK.

We tried to follow how it works in the PHP SDK (relevant function here: https://github.com/Learnosity/learnosity-sdk-php/blob/master/src/LearnositySdk/Request/Init.php#L174)

@norvin-yecla-lrn norvin-yecla-lrn force-pushed the CAT-66/feature/add-telemetry branch from 2564a64 to e1899fe Compare July 29, 2019 06:11
/// <returns>The version number</returns>
public string getSdkVersion()
{
return GetType().Assembly.GetName().Version.ToString();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will return a string with four sections separated by periods. i.e. 0.0.8.0

@norvin-yecla-lrn norvin-yecla-lrn force-pushed the CAT-66/feature/add-telemetry branch from b117dc2 to 9855d6c Compare July 30, 2019 00:41
@norvin-yecla-lrn norvin-yecla-lrn force-pushed the CAT-66/feature/add-telemetry branch from 9855d6c to a7b1e35 Compare July 31, 2019 05:44
/// </summary>
/// <returns>A signature hash for the request authentication</returns>
private string generateSignature()
public string generateSignature()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turned to public so we can use it in the tests. In Java SDK this is also a public method.

Copy link
Contributor

@rhiannon-eldridge-lrn rhiannon-eldridge-lrn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just needs some minor changes to be consistent with the existing code, otherwise looks great to me.

@norvin-yecla-lrn norvin-yecla-lrn merged commit 3a350a5 into master Aug 7, 2019
@norvin-yecla-lrn norvin-yecla-lrn deleted the CAT-66/feature/add-telemetry branch August 7, 2019 01:26
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.

5 participants