Skip to content

Ensure Kibana client reconnects. #2421

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.

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

Merged
merged 7 commits into from
Jul 11, 2019
Merged

Conversation

simitt
Copy link
Contributor

@simitt simitt commented Jul 10, 2019

Implement Kibana client wrapper that takes care of obtaining a valid
Kibana client, in case client cannot be obtained on startup.
Align returned response body to whether or not the user sends a secret token.
Log full error messages.

fixes #2371

Implement Kibana client wrapper that takes care of obtaining a valid
Kibana client, in case client cannot be obtained on startup.

fixes elastic#2371
@simitt simitt added this to the 7.3 milestone Jul 10, 2019
@simitt simitt requested a review from graphaelli July 10, 2019 17:51
@simitt simitt self-assigned this Jul 10, 2019
Copy link
Member

@graphaelli graphaelli left a comment

Choose a reason for hiding this comment

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

This is great, kibana connection was reestablished after being broken as expected. Some strange response bodies though:

kibana stopped after start:

$ curl -H 'Accept: */*' -i http://localhost:8200/config/v1/agents?service.name=foo
HTTP/1.1 503 Service Unavailable
Cache-Control: max-age=300, must-revalidate
Content-Type: application/json
X-Content-Type-Options: nosniff
Date: Wed, 10 Jul 2019 20:06:08 GMT
Content-Length: 35

"sending request to kibana failed"

kibana partially up - Kibana server is not ready yet

$ curl -H 'Accept: */*' -i http://localhost:8200/config/v1/agents?service.name=foo
HTTP/1.1 503 Service Unavailable
Cache-Control: max-age=300, must-revalidate
Content-Type: application/json
X-Content-Type-Options: nosniff
Date: Wed, 10 Jul 2019 20:06:15 GMT
Content-Length: 3

""

no config available:

$ curl -H 'Accept: */*' -i http://localhost:8200/config/v1/agents?service.name=foo
HTTP/1.1 404 Not Found
Cache-Control: max-age=300, must-revalidate
Content-Type: application/json
X-Content-Type-Options: nosniff
Date: Wed, 10 Jul 2019 20:06:50 GMT
Content-Length: 22

"no config found for"

Those double quoted, non json bodies don't seem right, or is that what we agreed upon for now?

if err != nil {
b = []byte(fmt.Sprintf("%+v", body))
}
b = append(b, "\n"...)
Copy link
Member

Choose a reason for hiding this comment

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

not for now, but we should see if it's worth avoiding creation of b and write directly instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

totally agree


var (
mockCfg = common.MustNewConfigFrom(`{"enabled": "false", "host": "non-existing"}`)
mockBody = ioutil.NopCloser(convert.ToReader(`{"response": "ok"}`))
Copy link
Member

Choose a reason for hiding this comment

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

this is the first I've seen ToReader - it should probably just return a ioutil.NopCloser since we're wrapping that everywhere it's used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I only moved this, so didn't check in detail. But there are other places where we don't wrap it e.g. agentcfg/fetch.go#L63.

@simitt
Copy link
Contributor Author

simitt commented Jul 10, 2019

Those double quoted, non json bodies don't seem right, or is that what we agreed upon for now?

That's actually the same behavior in master, I haven't touched anything related to how the messages are rendered. I can also fix it within the PR. What do you think about e.g { "error": "no config found" }?

@simitt simitt merged commit a9919ab into elastic:master Jul 11, 2019
simitt added a commit to simitt/apm-server that referenced this pull request Jul 11, 2019
Implement Kibana client wrapper that takes care of obtaining a valid
Kibana client, in case client cannot be obtained on startup.
Align returned response body to whether or not the user sends a secret token.
Log full error messages.

fixes elastic#2371
simitt added a commit to simitt/apm-server that referenced this pull request Jul 11, 2019
Implement Kibana client wrapper that takes care of obtaining a valid
Kibana client, in case client cannot be obtained on startup.
Align returned response body to whether or not the user sends a secret token.
Log full error messages.

fixes elastic#2371
simitt added a commit to simitt/apm-server that referenced this pull request Jul 11, 2019
Implement Kibana client wrapper that takes care of obtaining a valid
Kibana client, in case client cannot be obtained on startup.
Align returned response body to whether or not the user sends a secret token.
Log full error messages.

fixes elastic#2371
graphaelli pushed a commit that referenced this pull request Jul 11, 2019
Implement Kibana client wrapper that takes care of obtaining a valid
Kibana client, in case client cannot be obtained on startup.
Align returned response body to whether or not the user sends a secret token.
Log full error messages.

fixes #2371
graphaelli pushed a commit that referenced this pull request Jul 11, 2019
Implement Kibana client wrapper that takes care of obtaining a valid
Kibana client, in case client cannot be obtained on startup.
Align returned response body to whether or not the user sends a secret token.
Log full error messages.

fixes #2371
@jalvz jalvz mentioned this pull request Jul 16, 2019
7 tasks
@simitt simitt deleted the 2371-kibana-retry branch July 23, 2019 06:24
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.

Agent Configuration unavailable without Kibana available at startup
2 participants