Skip to content

Add test for TCP Metrics#741

Merged
rshriram merged 4 commits intoistio:masterfrom
douglas-reid:tcp_tests
Sep 14, 2017
Merged

Add test for TCP Metrics#741
rshriram merged 4 commits intoistio:masterfrom
douglas-reid:tcp_tests

Conversation

@douglas-reid
Copy link
Copy Markdown
Contributor

@douglas-reid douglas-reid commented Sep 11, 2017

This PR adds a basic test to check that TCP metrics can be generated for TCP services. It uses the bookinfo sample app, including the mongodb service.

It also adds:

  • attributes to the required manifest
  • rules and metrics definitions to the standard-metrics (and prometheus)
  • the kube-inject for the db rules.

Release note:

NONE

@istio-merge-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
We suggest the following additional approver: rshriram

Assign the PR to them by writing /assign @rshriram in a comment when ready.

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

@douglas-reid PR needs rebase

@istio-merge-robot istio-merge-robot added needs-rebase Indicates a PR needs to be rebased before being merged and removed needs-rebase Indicates a PR needs to be rebased before being merged labels Sep 14, 2017
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.

new line please

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.

yup

@istio-testing
Copy link
Copy Markdown
Collaborator

istio-testing commented Sep 14, 2017

@douglas-reid: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
prow/new-e2e-rbac_no_auth.sh 7d75cd212ac2ca5e99209a6332fc05ec399292cf link /test new-e2e-rbac_no_auth
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.

@rshriram rshriram merged commit 9c53d52 into istio:master Sep 14, 2017
Copy link
Copy Markdown
Member

@ldemailly ldemailly left a comment

Choose a reason for hiding this comment

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

would be good to stick to 2 reviews in the next couple of days

}

func TestTcpMetrics(t *testing.T) {
if err := replaceRouteRule(tcpDbRule); err != nil {
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.

ah nice, this is the test mentioned in the other PR, sorry somehow I didn't see that PR until now
(don't hesitate to ping me in the future if I don't comment on a PR before it gets merged)

fatalf(t, "Could not find metric value: %v", err)
}
t.Logf("tcp_bytes_sent: %f", got)
want := float64(1)
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 reply is bigger than 1 byte no ?

fatalf(t, "Could not find metric value: %v", err)
}
t.Logf("tcp_bytes_received: %f", got)
if got < want {
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.

same

}
glog.Infof("promvalue := %s", value.String())

got, err := vectorValue(value, map[string]string{})
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.

shouldn't there be a subtraction of the previous value here too ?
(maybe that could be a function to delta the stats - or I missed a refactor and it already does that ?)

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.

yup

rshriram pushed a commit that referenced this pull request Oct 30, 2017
* Add test for TCP Metrics

* versioned selection

* Do not duplicate destination attributes

* no more target


Former-commit-id: 9c53d52
vbatts pushed a commit to vbatts/istio that referenced this pull request Oct 31, 2017
* Add test for TCP Metrics

* versioned selection

* Do not duplicate destination attributes

* no more target


Former-commit-id: 9c53d52
mandarjog pushed a commit that referenced this pull request Nov 2, 2017
* Add test for TCP Metrics

* versioned selection

* Do not duplicate destination attributes

* no more target


Former-commit-id: 9c53d52
howardjohn pushed a commit to howardjohn/istio that referenced this pull request Jan 12, 2020
* fix iop status update issue.

* fix lint.

* update status with instance upadter.

* add back the status subresource.

* move status out of spec.
0x01001011 pushed a commit to thedemodrive/istio that referenced this pull request Jul 16, 2020
* Introduce forward looking MCP enhancements

The initial version of Mesh Configuration Protocol (MCP) was
introduced to decouple Pilot/Mixer from the k8s kube-apiserver. These
enhancements address additional forward looking requirements as we
bring Galley and the MCP API to beta quality.

* Enable alternative control topologies where the source of
  configuration is not publicly accessible.

* Provide a feedback mechanism to report the observed config state to
  the user (e.g via CRD status).

* Improve performance at scale (e.g. Enterprise use case)

* Rationalize the resource model

The intent is to introduce these backwards incompatible API changes
now before Galley and MCP ship as beta quality and on-by-default.

Rationalization of the resource model and incremental improvements can
be implemented immediately as they effect the contract between Pilot
and Galley. Alternative control topologies and feedback/status are
inherently new features and can be implemented later with the same
APIs without concerns of breaking compatibility.

Design proposal: https://goo.gl/RTKMwF

* fix linter error

* add missing generated file

* proto-commit

* remove python/istio_api/mcp/v1alpha1/envelope_pb2.py

* s/envelope/resource

* s/client/node

* make proto-commit

* fix comments

* add system_version_info for compatibility with non-incremental MCP

* address review comments

* s/node/sink_node

* address more review comments

* update resource name documentation
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.

6 participants