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

Support Envoy {AccessLog,Metrics}Service API V3 #5872

Closed
wants to merge 6 commits into from
Closed

Conversation

kezhenxu94
Copy link
Member

Upgrade Envoy API to V3

@kezhenxu94 kezhenxu94 added backend OAP backend related. dependencies Pull requests that update a dependency file labels Nov 20, 2020
@codecov
Copy link

codecov bot commented Nov 20, 2020

Codecov Report

Merging #5872 (0f859f7) into master (1b1f085) will decrease coverage by 1.32%.
The diff coverage is 4.54%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #5872      +/-   ##
============================================
- Coverage     51.34%   50.02%   -1.33%     
+ Complexity     3511     3428      -83     
============================================
  Files          1693      954     -739     
  Lines         35832    23321   -12511     
  Branches       3958     2269    -1689     
============================================
- Hits          18399    11666    -6733     
+ Misses        16492    10705    -5787     
- Partials        941      950       +9     
Impacted Files Coverage Δ Complexity Δ
...er/receiver/envoy/AccessLogServiceGRPCHandler.java 0.00% <0.00%> (-19.24%) 0.00 <0.00> (-2.00)
...erver/receiver/envoy/MetricServiceGRPCHandler.java 0.00% <0.00%> (-12.17%) 0.00 <0.00> (-2.00)
...oap/server/receiver/envoy/als/ALSHTTPAnalysis.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...server/receiver/envoy/als/AbstractALSAnalyzer.java 0.00% <ø> (-50.00%) 0.00 <0.00> (ø)
...er/receiver/envoy/als/LogEntry2MetricsAdapter.java 0.00% <0.00%> (-50.46%) 0.00 <0.00> (ø)
...g/oap/server/receiver/envoy/als/ProtoMessages.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...r/envoy/als/k8s/K8sALSServiceMeshHTTPAnalysis.java 0.00% <0.00%> (-67.65%) 0.00 <0.00> (ø)
...iver/envoy/als/mx/MetaExchangeALSHTTPAnalyzer.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
.../server/receiver/envoy/als/wrapper/Identifier.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
.../receiver/envoy/AccessLogServiceV2GRPCHandler.java 25.00% <25.00%> (ø) 1.00 <1.00> (?)
... and 1199 more

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 1b1f085...0f859f7. Read the comment docs.

@wu-sheng
Copy link
Member

Seems like, there was local UT for old format? As we have the e2e, @hanahmily should we consider removing those? And Make sure e2e passed.

@hanahmily
Copy link
Contributor

Seems like, there was local UT for old format? As we have the e2e, @hanahmily should we consider removing those? And Make sure e2e passed.

make sense to me.

@kezhenxu94
Copy link
Member Author

Only 1.8.0+ includes this patch to bootstrap Envoy with v3, while due to istio/istio#28956 , I think we have to wait for the latter one being cherry-picked in 1.8.x (1.8.1 doesn't include it still)

@kezhenxu94 kezhenxu94 force-pushed the issue/5860 branch 2 times, most recently from fb1c29e to 1cf24ef Compare December 14, 2020 15:10
@kezhenxu94 kezhenxu94 changed the title Upgrade Envoy API to V3 Support Envoy {AccessLog,Metrics}Service API V3 Dec 14, 2020
try {
double value;
long timestamp = 0;
switch (family.getType()) {
Copy link

Choose a reason for hiding this comment

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

NULL_DEREFERENCE: object returned by family.getType() could be null and is dereferenced at line 117.

@kezhenxu94 kezhenxu94 force-pushed the issue/5860 branch 2 times, most recently from a1c543b to 1643c99 Compare December 16, 2020 13:24
@kezhenxu94 kezhenxu94 marked this pull request as ready for review January 1, 2021 09:39
@kezhenxu94
Copy link
Member Author

Close in favor of #6116

@kezhenxu94 kezhenxu94 closed this Jan 1, 2021
@kezhenxu94 kezhenxu94 deleted the issue/5860 branch January 1, 2021 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend OAP backend related. dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade Envoy API to V3
3 participants