Skip to content

Add Datadog tracing to proxy config (release-1.1)#822

Merged
rshriram merged 1 commit intoistio:release-1.1from
DataDog:cgilmour/datadog-tracer-r1.1
Mar 21, 2019
Merged

Add Datadog tracing to proxy config (release-1.1)#822
rshriram merged 1 commit intoistio:release-1.1from
DataDog:cgilmour/datadog-tracer-r1.1

Conversation

@cgilmour
Copy link
Copy Markdown
Contributor

@cgilmour cgilmour commented Feb 26, 2019

This was already submitted and accepted on master, but due to other changes, needs to be resubmitted on release-1.1

Signed-off-by: Caleb Gilmour caleb.gilmour@datadoghq.com

@googlebot googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Feb 26, 2019
@istio-testing
Copy link
Copy Markdown
Collaborator

istio-testing commented Feb 26, 2019

Hi @cgilmour. Thanks for your PR.

I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@cgilmour cgilmour mentioned this pull request Feb 27, 2019
Closed
@rshriram
Copy link
Copy Markdown
Member

rshriram commented Feb 28, 2019

/ok-to-test

@istio-testing istio-testing added ok-to-test Set this label allow normal testing to take place for a PR not submitted by an Istio org member. and removed needs-ok-to-test labels Feb 28, 2019
@duderino
Copy link
Copy Markdown

duderino commented Feb 28, 2019

@rshriram @douglas-reid you guys already lgtm'd this on against master. Could you lgtm it for the release-1.1 branch too? The master PR for your reference: #797

@cgilmour given how close we are to cutting 1.1, I'd like to merge this immediately after so it would ship in 1.1.1.

@cgilmour
Copy link
Copy Markdown
Contributor Author

cgilmour commented Feb 28, 2019

Sounds good, @duderino

@costinm
Copy link
Copy Markdown
Contributor

costinm commented Mar 1, 2019

/lgtm

@costinm
Copy link
Copy Markdown
Contributor

costinm commented Mar 1, 2019

/approve

douglas-reid
douglas-reid approved these changes Mar 1, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@douglas-reid douglas-reid left a comment

Choose a reason for hiding this comment

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

/lgtm

@istio-testing
Copy link
Copy Markdown
Collaborator

istio-testing commented Mar 1, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgilmour, costinm, douglas-reid

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@cgilmour cgilmour force-pushed the cgilmour/datadog-tracer-r1.1 branch from f24e1d7 to 2e27b99 Compare March 7, 2019 22:38
@istio-testing
Copy link
Copy Markdown
Collaborator

istio-testing commented Mar 7, 2019

New changes are detected. LGTM label has been removed.

@istio-testing istio-testing removed the lgtm label Mar 7, 2019
@cgilmour cgilmour force-pushed the cgilmour/datadog-tracer-r1.1 branch from 2e27b99 to 0236aa7 Compare March 10, 2019 20:56
@cgilmour
Add Datadog tracing to proxy config
4e513db 
Signed-off-by: Caleb Gilmour <caleb.gilmour@datadoghq.com>
@cgilmour cgilmour force-pushed the cgilmour/datadog-tracer-r1.1 branch from 0236aa7 to 4e513db Compare March 18, 2019 02:59
@rshriram
Copy link
Copy Markdown
Member

rshriram commented Mar 19, 2019

is this feature in istio/istio release-1.1 branch already?

@cgilmour
Copy link
Copy Markdown
Contributor Author

cgilmour commented Mar 19, 2019

No, it's on master from an earlier PR.

@cgilmour
Copy link
Copy Markdown
Contributor Author

cgilmour commented Mar 20, 2019

Can I get an OK on this? It's needed to produce a PR against istio release-1.1 branch also.
(A rebase of istio/istio#11855)

@rshriram
Copy link
Copy Markdown
Member

rshriram commented Mar 21, 2019

Ah so you want this to go into release-1.1 ? it was not immediately clear from "but due to other changes, needs to be resubmitted on release-1.1" that this was a backport as well. Thats why I was asking if this was in istio/istio or not. I am not sure if its too late to get into 1.1 as our feature freeze happened a month ago before this PR. But if the change is straightforward, we might be able to get it in.

@rshriram
Copy link
Copy Markdown
Member

rshriram commented Mar 21, 2019

thoughts @duderino ? this is similar to lightstep and such, which landed a while ago

@rshriram
Copy link
Copy Markdown
Member

rshriram commented Mar 21, 2019

nm.. I see @duderino 's response above

@rshriram rshriram merged commit 4405784 into istio:release-1.1 Mar 21, 2019
@cgilmour
Copy link
Copy Markdown
Contributor Author

cgilmour commented Mar 21, 2019

Thanks!

nacx pushed a commit to nacx/api that referenced this pull request Apr 15, 2020
@tetrate-ci
Redo Shriram's PR (istio#822)
63a1181 
I messed up @rshriram's PR here: tetrateio/tetrate#821 🙇

Signed-off-by: Dhi Aurrahman <dio@tetrate.io>

Mirrored from https://github.com/tetrateio/tetrate @ 91aff3348a70a563a464f36457f3b5044e3b63d1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@costinm costinm Awaiting requested review from costinm

@mandarjog mandarjog Awaiting requested review from mandarjog

2 more reviewers

@rshriram rshriram rshriram approved these changes

@douglas-reid douglas-reid douglas-reid approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@costinm costinm

@douglas-reid douglas-reid

Labels

cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. ok-to-test Set this label allow normal testing to take place for a PR not submitted by an Istio org member.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@cgilmour @istio-testing @rshriram @duderino @costinm @douglas-reid @googlebot