Skip to content

Re-enable pprof for mixer with args consistent with Pilot#3539

Merged
istio-merge-robot merged 2 commits intoistio:masterfrom
mandarjog:mixer_pprof
Feb 18, 2018
Merged

Re-enable pprof for mixer with args consistent with Pilot#3539
istio-merge-robot merged 2 commits intoistio:masterfrom
mandarjog:mixer_pprof

Conversation

@mandarjog
Copy link
Copy Markdown
Contributor

Enable pprof at http://localhost:9093/debug/pprof/* with command line args consistent with Pilot.

Signed-off-by: Mandar U Jog mjog@google.com

@mandarjog mandarjog requested a review from a team February 15, 2018 23:51
@mandarjog
Copy link
Copy Markdown
Contributor Author

/test istio-pilot-e2e

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you create a unit test for this? The server package has 100% coverage, it'd be nice to stay there :-)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

the default being true, it's covered

@yutongz
Copy link
Copy Markdown
Contributor

yutongz commented Feb 16, 2018

/test e2e-simple
/test e2e-bookInfo

We renamed e2e-smoke to e2e-bookInfo and added e2e-simple
ref: #3529

@douglas-reid
Copy link
Copy Markdown
Contributor

@mandarjog do we want some sort of integration test to validate this works as expected to go along with the unit test?

@douglas-reid
Copy link
Copy Markdown
Contributor

/test istio-pilot-e2e
/test e2e-bookInfo

@ldemailly
Copy link
Copy Markdown
Member

/lgtm
#3587 to see what's up with the linter error

@ldemailly
Copy link
Copy Markdown
Member

sorry to put that on you mandar but can you fix the linter issues (#3471 is the cause of this problem, master is broken because the pr that added the lint errors was green because the linters got 'fixed' in between)
otherwise I'll get to it after I fix #3585

@ldemailly
Copy link
Copy Markdown
Member

linter are fixed (#3591 ) so you should be good to go

/test istio-pilot-e2e

Signed-off-by: Mandar U Jog <mjog@google.com>
Signed-off-by: Mandar U Jog <mjog@google.com>
@istio-merge-robot
Copy link
Copy Markdown

/lgtm cancel //PR changed after LGTM, removing LGTM. @ldemailly @mandarjog

@ldemailly
Copy link
Copy Markdown
Member

/lgtm

@istio-merge-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ldemailly

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@istio-merge-robot
Copy link
Copy Markdown

/test all [submit-queue is verifying that this PR is safe to merge]

@istio-merge-robot
Copy link
Copy Markdown

Automatic merge from submit-queue.

@istio-merge-robot istio-merge-robot merged commit 2c31a49 into istio:master Feb 18, 2018
@istio-testing
Copy link
Copy Markdown
Collaborator

@mandarjog: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
prow/istio-pilot-e2e.sh 2a6ebcf link /test istio-pilot-e2e
Details

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. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants