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

[WIP] AO-14906: remove the old udp reporter #135

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jiwen624
Copy link
Contributor

@jiwen624 jiwen624 commented Dec 9, 2019

WIP.

Currently, there are a few fields, e.g., the custom metrics, are attached to the gRPC reporter which makes it not just a transport layer as indicated by its name. On the other hand, the UDP reporter is just a thin layer reporting events/metrics via UDP.

In addition, a few global variables exist in the package to do the protocol-independent tasks. For example, the global settings and configurations. These global variables make testing a tough job. What's more, in order to add a new feature we may have to either implement it in each reporter or use another global variable.

For ease of testing, we're trying to eliminate the global variables by attaching the settings and configurations to the reporter and make the (unified) reporter protocol-agnostic. The transport layer (either gRPC or UDP or something else) could be a field of the reporter which only does the final data transportation work.

The first step to achieve the above goal is to remove the old UDP reporter. It will be added back (in a different way) in case it's needed in the future.

@jiwen624 jiwen624 changed the title AO-14906: Refactor old udp reporter by removing it first [WIP] AO-14906: Refactor old udp reporter by removing it first Dec 9, 2019
@codecov-io
Copy link

Codecov Report

Merging #135 into master will increase coverage by 0.89%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #135      +/-   ##
==========================================
+ Coverage   81.91%   82.81%   +0.89%     
==========================================
  Files          37       36       -1     
  Lines        4590     4538      -52     
==========================================
- Hits         3760     3758       -2     
+ Misses        705      656      -49     
+ Partials      125      124       -1
Impacted Files Coverage Δ
v1/ao/internal/config/config.go 83.37% <ø> (-0.19%) ⬇️
v1/ao/internal/config/wrappers.go 33.33% <ø> (ø) ⬆️
v1/ao/internal/reporter/reporter.go 81.9% <100%> (+1.53%) ⬆️
v1/ao/internal/config/validators.go 96% <100%> (ø) ⬆️
v1/ao/internal/reporter/reporter_grpc.go 69.67% <0%> (+0.29%) ⬆️

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 963bee1...f0837e7. Read the comment docs.

@jiwen624 jiwen624 changed the title [WIP] AO-14906: Refactor old udp reporter by removing it first [WIP] AO-14906: remove the old udp reporter Dec 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants