Skip to content
This repository has been archived by the owner on Oct 3, 2023. It is now read-only.

Addition of Stats Exporter for Microsoft Azure #795

Merged

Conversation

jimmy-long
Copy link
Contributor

@jimmy-long jimmy-long commented Apr 17, 2020

This is in relation to issue #751 which I opened for my team's senior design project at NC State University.

jwlongnc and others added 30 commits February 10, 2020 11:22
Started by adding a few basic options that can be configured.
Added a timer to handle batching of metric exports.
Started integration of App Insights SDK.
@mayurkale22
Copy link
Member

@mayurkale22 The authors that the bot needs are just different accounts for the same 2 people, Jimmy and Tony. Any chance you can help get the cla label set to yes?

For now, I have manually verified the CLA.

@mayurkale22
Copy link
Member

Please fix the build and unresolved comments.

@googlebot
Copy link

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: no and removed cla: yes labels May 1, 2020
@codecov-io
Copy link

codecov-io commented May 1, 2020

Codecov Report

Merging #795 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #795   +/-   ##
=======================================
  Coverage   95.42%   95.42%           
=======================================
  Files         148      148           
  Lines       10743    10743           
  Branches      798      798           
=======================================
  Hits        10252    10252           
  Misses        491      491           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 01fa6ae...01fa6ae. Read the comment docs.

@jimmy-long
Copy link
Contributor Author

I went ahead and resolved comments that were a simple fix (adding license header, version bump, etc). Left two open that were related to design and testing. Happy to address any other concerns! Thanks.

@gdkerr
Copy link

gdkerr commented May 9, 2020

@googlebot I signed it

@cdietschrun
Copy link

@mayurkale22 Thanks for the approve-The only CLA bump here is Gaven, but it's from an account I don't think he has access to (since it was with his university account and he's graduated). @gdkerr is Gaven Kerr's other account; I think you can probably just manually set the CLA as signed for him, or reach out to him to confirm.

@googlebot
Copy link

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

@mayurkale22 mayurkale22 merged commit e272ccc into census-instrumentation:master May 12, 2020
@mayurkale22
Copy link
Member

@mayurkale22 Thanks for the approve-The only CLA bump here is Gaven, but it's from an account I don't think he has access to (since it was with his university account and he's graduated). @gdkerr is Gaven Kerr's other account; I think you can probably just manually set the CLA as signed for him, or reach out to him to confirm.

I have manually verified the CLA and merged the PR. Let me know if you are waiting for the release.

@cdietschrun
Copy link

Great news thanks! We'd like to pick up this release but it's not urgent (order of weeks would be our integration time)

@cdietschrun
Copy link

@mayurkale22 I'm just curious if you have a timeline for package release including this exporter? We have some resources that I would love to assign to integrate this coming soon.

@mayurkale22
Copy link
Member

@mayurkale22 I'm just curious if you have a timeline for package release including this exporter? We have some resources that I would love to assign to integrate this coming soon.

Apologies for the delay. I have cut the new release. See: https://www.npmjs.com/package/@opencensus/exporter-azure

Noticed two things:

  1. README.md file is missing
  2. I had to change publishConfig to cut the release. pls release (chore(exporter-azure): add publishConfig #814)

@cdietschrun
Copy link

No need to apologize, this is great! Thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants