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

Add a heartbeat-specific version property to the metrics sent as hear… #782

Merged
merged 5 commits into from
Apr 19, 2018

Conversation

d3r3kk
Copy link
Contributor

@d3r3kk d3r3kk commented Apr 19, 2018

Add a heartbeat-specific version string to the properties of the metrics sent as heartbeats.

  • Design discussion (none needed - issue raised in informal discussions)
  • Changes in public surface reviewed
    • no change to public API
  • CHANGELOG.md
    • probably unnecessary for this small change, let me know if you still want it in there!

@d3r3kk d3r3kk requested a review from SergeyKanzhelev April 19, 2018 19:13
@d3r3kk d3r3kk self-assigned this Apr 19, 2018
@d3r3kk d3r3kk added this to the 2.7-Beta1 milestone Apr 19, 2018
@SergeyKanzhelev
Copy link
Contributor

Copy link
Contributor

@cijothomas cijothomas left a comment

Choose a reason for hiding this comment

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

consider using just hb instead of hbeat to save space

/// <summary>
/// Value for property indicating 'app insights version' related specifically to heartbeats.
/// </summary>
private static string sdkVersionPropertyValue = SdkVersionUtils.GetSdkVersion("hbeat:");
Copy link
Contributor

Choose a reason for hiding this comment

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

just 'hb' is sufficient to save space.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. Will make the change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Please accept PR Microsoft/ApplicationInsights-Home#214 along with this one.

@d3r3kk d3r3kk merged commit af7f4cf into develop Apr 19, 2018
@d3r3kk d3r3kk deleted the dekeeler/hbeat-ver branch April 19, 2018 21:34
@d3r3kk
Copy link
Contributor Author

d3r3kk commented Apr 19, 2018

@cijothomas or @SergeyKanzhelev could you please merge Microsoft/ApplicationInsights-Home#214 to coincide with this change? Thanks.

TimothyMothra pushed a commit that referenced this pull request Oct 25, 2019
dev to master for 2.6.0-beta1
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.

3 participants