Skip to content

stats(native): fixes to align with wasm stats#4125

Merged
istio-testing merged 2 commits intoistio:masterfrom
kyessenov:more_test_cases
Oct 19, 2022
Merged

stats(native): fixes to align with wasm stats#4125
istio-testing merged 2 commits intoistio:masterfrom
kyessenov:more_test_cases

Conversation

@kyessenov
Copy link
Contributor

Signed-off-by: Kuat Yessenov kuat@google.com

Fix an error in parsing in Wasm stats for endpoint encoding (parts == 4 leads to crash).
Adds fallbacks to native stats for endpoint encoding, blackhole/passthrough.
Handle empty values properly (should be unknown).
Finish CEL expression support.

Signed-off-by: Kuat Yessenov <kuat@google.com>
@kyessenov kyessenov requested a review from a team October 19, 2022 18:17
@istio-testing istio-testing added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 19, 2022
@@ -1,8 +1,5 @@
field_separator: ";.;"
definitions:
- name: requests_total
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is requests_total deleted?

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 test case was changing the value for a standard metric. I don't think we want to support this feature.

Signed-off-by: Kuat Yessenov <kuat@google.com>
@kyessenov
Copy link
Contributor Author

/retest

@kyessenov
Copy link
Contributor Author

/retest

@kyessenov kyessenov closed this Oct 19, 2022
@kyessenov kyessenov reopened this Oct 19, 2022
@kyessenov
Copy link
Contributor Author

/retest

@istio-testing istio-testing merged commit 2063df4 into istio:master Oct 19, 2022
@kyessenov
Copy link
Contributor Author

/cherrypick release-1.16

@kyessenov kyessenov added the cherrypick/release-1.16 Set this label on a PR to auto-merge it to the release-1.16 branch label Oct 20, 2022
@istio-testing
Copy link
Collaborator

@kyessenov: #4125 failed to apply on top of branch "release-1.16":

Applying: fixes
Using index info to reconstruct a base tree...
M	extensions/common/metadata_object.cc
M	extensions/common/metadata_object_test.cc
M	source/extensions/filters/http/istio_stats/BUILD
M	source/extensions/filters/http/istio_stats/istio_stats.cc
Falling back to patching base and 3-way merge...
Auto-merging source/extensions/filters/http/istio_stats/istio_stats.cc
CONFLICT (content): Merge conflict in source/extensions/filters/http/istio_stats/istio_stats.cc
Auto-merging source/extensions/filters/http/istio_stats/BUILD
CONFLICT (content): Merge conflict in source/extensions/filters/http/istio_stats/BUILD
Auto-merging extensions/common/metadata_object_test.cc
CONFLICT (content): Merge conflict in extensions/common/metadata_object_test.cc
Auto-merging extensions/common/metadata_object.cc
CONFLICT (content): Merge conflict in extensions/common/metadata_object.cc
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 fixes
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Details

In response to this:

/cherrypick release-1.16

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@istio-testing
Copy link
Collaborator

@kyessenov: new issue could not be created for failed cherrypick: status code 410 not one of [201], body: {"message":"Issues are disabled for this repo","documentation_url":"https://docs.github.com/v3/issues/"}

Details

In response to this:

/cherrypick release-1.16

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@istio-testing
Copy link
Collaborator

In response to a cherrypick label: #4125 failed to apply on top of branch "release-1.16":

Applying: fixes
Using index info to reconstruct a base tree...
M	extensions/common/metadata_object.cc
M	extensions/common/metadata_object_test.cc
M	source/extensions/filters/http/istio_stats/BUILD
M	source/extensions/filters/http/istio_stats/istio_stats.cc
Falling back to patching base and 3-way merge...
Auto-merging source/extensions/filters/http/istio_stats/istio_stats.cc
CONFLICT (content): Merge conflict in source/extensions/filters/http/istio_stats/istio_stats.cc
Auto-merging source/extensions/filters/http/istio_stats/BUILD
CONFLICT (content): Merge conflict in source/extensions/filters/http/istio_stats/BUILD
Auto-merging extensions/common/metadata_object_test.cc
CONFLICT (content): Merge conflict in extensions/common/metadata_object_test.cc
Auto-merging extensions/common/metadata_object.cc
CONFLICT (content): Merge conflict in extensions/common/metadata_object.cc
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 fixes
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

@istio-testing
Copy link
Collaborator

In response to a cherrypick label: new issue could not be created for failed cherrypick: status code 410 not one of [201], body: {"message":"Issues are disabled for this repo","documentation_url":"https://docs.github.com/v3/issues/"}

kyessenov added a commit to kyessenov/proxy that referenced this pull request Oct 20, 2022
* fixes

Signed-off-by: Kuat Yessenov <kuat@google.com>

* handle all wasm properties

Signed-off-by: Kuat Yessenov <kuat@google.com>

Signed-off-by: Kuat Yessenov <kuat@google.com>
istio-testing pushed a commit that referenced this pull request Oct 21, 2022
* stats(native): use SAN namespace by default (#4117)

* stats(native): use SAN namespace by default

Signed-off-by: Kuat Yessenov <kuat@google.com>

* format

Signed-off-by: Kuat Yessenov <kuat@google.com>

Signed-off-by: Kuat Yessenov <kuat@google.com>

* workload metadata: harden parsing (#4120)

* workload metadata: harden parsing

Signed-off-by: Kuat Yessenov <kuat@google.com>

* fix test

Signed-off-by: Kuat Yessenov <kuat@google.com>

* fix test

Signed-off-by: Kuat Yessenov <kuat@google.com>

Signed-off-by: Kuat Yessenov <kuat@google.com>

* stats(native): fixes to align with wasm stats (#4125)

* fixes

Signed-off-by: Kuat Yessenov <kuat@google.com>

* handle all wasm properties

Signed-off-by: Kuat Yessenov <kuat@google.com>

Signed-off-by: Kuat Yessenov <kuat@google.com>

Signed-off-by: Kuat Yessenov <kuat@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cherrypick/release-1.16 Set this label on a PR to auto-merge it to the release-1.16 branch size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants