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.

Sign up for GitHub

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

Jump to bottom

fix disabling ALL_METRICS does not work as expected #43179

Merged
merged 5 commits into from Feb 16, 2023
Merged

fix disabling ALL_METRICS does not work as expected #43179

merged 5 commits into from Feb 16, 2023

Conversation

zirain
Copy link
Member

@zirain zirain commented Feb 7, 2023 •
edited by istio-policy-bot

Signed-off-by: hejianpeng hejianpeng2@huawei.com

Please provide a description of this PR:

fixes: #43178

@zirain zirain requested a review from a team as a code owner February 7, 2023 12:34
@istio-testing istio-testing added do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 7, 2023
@zirain
fix disabling ALL_METERS does not work as expected
ec9976f 
Signed-off-by: hejianpeng <hejianpeng2@huawei.com>
@zirain zirain requested review from a team as code owners February 8, 2023 01:27
@zirain zirain changed the title [WIP]fix disabling ALL_METERS does not work as expected fix disabling ALL_METERS does not work as expected Feb 8, 2023
@istio-testing istio-testing removed the do-not-merge/work-in-progress Block merging of a PR because it isn't ready yet. label Feb 8, 2023
@zirain zirain added cherrypick/release-1.15 cherrypick/release-1.16 Set this label on a PR to auto-merge it to the release-1.16 branch cherrypick/release-1.17 Set this label on a PR to auto-merge it to the release-1.17 branch labels Feb 9, 2023
kyessenov
kyessenov requested changes Feb 10, 2023
View reviewed changes
Copy link
Contributor

@kyessenov kyessenov left a comment

Choose a reason for hiding this comment

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

LGTM, some style improvement.

releasenotes/notes/43179.yaml Outdated Show resolved Hide resolved
pilot/pkg/model/telemetry.go Outdated Show resolved Hide resolved
pilot/pkg/model/telemetry.go Outdated Show resolved Hide resolved
@zirain zirain changed the title fix disabling ALL_METERS does not work as expected fix disabling ALL_METRICS does not work as expected Feb 11, 2023
kyessenov
kyessenov approved these changes Feb 13, 2023
View reviewed changes
Copy link
Contributor

@kyessenov kyessenov left a comment

Choose a reason for hiding this comment

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

I'd like @lei-tang to take a look since we're close to a release.

GregHanson
GregHanson approved these changes Feb 15, 2023
View reviewed changes
Copy link
Member

@GregHanson GregHanson left a comment

Choose a reason for hiding this comment

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

release note looks good

ericvn
ericvn approved these changes Feb 15, 2023
View reviewed changes
Copy link
Contributor

@ericvn ericvn left a comment

Choose a reason for hiding this comment

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

Test and RN look good.

@istio-testing istio-testing merged commit 7389255 into istio:master Feb 16, 2023
@istio-testing
Copy link
Collaborator

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

Applying: fix disabling ALL_METERS does not work as expected
Using index info to reconstruct a base tree...
M	pilot/pkg/model/telemetry.go
Falling back to patching base and 3-way merge...
Auto-merging pilot/pkg/model/telemetry.go
CONFLICT (content): Merge conflict in pilot/pkg/model/telemetry.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 fix disabling ALL_METERS does not work as expected
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 istio-testing mentioned this pull request Feb 16, 2023
Closed
@istio-testing
Copy link
Collaborator

In response to a cherrypick label: new issue created for failed cherrypick: #43392

@istio-testing
Copy link
Collaborator

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

Applying: fix disabling ALL_METERS does not work as expected
Using index info to reconstruct a base tree...
M	pilot/pkg/model/telemetry.go
Falling back to patching base and 3-way merge...
Auto-merging pilot/pkg/model/telemetry.go
CONFLICT (content): Merge conflict in pilot/pkg/model/telemetry.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 fix disabling ALL_METERS does not work as expected
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 istio-testing mentioned this pull request Feb 16, 2023
Closed
@istio-testing
Copy link
Collaborator

In response to a cherrypick label: new issue created for failed cherrypick: #43393

@istio-testing istio-testing mentioned this pull request Feb 16, 2023
Merged
@istio-testing
Copy link
Collaborator

In response to a cherrypick label: new pull request created: #43394

@zirain zirain deleted the telemetry-metric-mode branch February 16, 2023 03:24
@zirain zirain mentioned this pull request Feb 16, 2023
Merged
zirain added a commit to zirain/istio that referenced this pull request Feb 16, 2023
@zirain
fix disabling ALL_METRICS does not work as expected (istio#43179)
df89644 
* fix disabling ALL_METERS does not work as expected

Signed-off-by: hejianpeng <hejianpeng2@huawei.com>

* fix lint

* add release notes

* address kuat's comment

* update release notes

---------

Signed-off-by: hejianpeng <hejianpeng2@huawei.com>
istio-testing pushed a commit that referenced this pull request Feb 16, 2023
@zirain
fix disabling ALL_METRICS does not work as expected (#43179) (#43395)
0b4b03f 
* fix disabling ALL_METERS does not work as expected



* fix lint

* add release notes

* address kuat's comment

* update release notes

---------

Signed-off-by: hejianpeng <hejianpeng2@huawei.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kyessenov kyessenov kyessenov approved these changes

@ericvn ericvn ericvn approved these changes

@hzxuzhonghu hzxuzhonghu hzxuzhonghu approved these changes

@GregHanson GregHanson GregHanson approved these changes

@lei-tang lei-tang lei-tang approved these changes

Assignees

@kyessenov kyessenov

@lei-tang lei-tang

Labels
area/extensions and telemetry cherrypick/release-1.16 Set this label on a PR to auto-merge it to the release-1.16 branch cherrypick/release-1.17 Set this label on a PR to auto-merge it to the release-1.17 branch size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Telemetry: disabling ALL_METERS does not work as expected
7 participants
@zirain @istio-testing @kyessenov @ericvn @hzxuzhonghu @GregHanson @lei-tang