Feature/elasticsearch new mapping #1313

AlmogBaku · 2016-09-23T20:15:52Z

This PR brings a lot of changes, most of them caused by refactoring the schema .
In order to allow effective usage of the ELK stack, we had to modify the schema so it'll work properly with Kibana(otherwise, it require development of a custom special dashboard! which is insane!)

This PR fixes the following issues:

Types per "metric family"(cpu/memory/network/fs/etc): each entry includes more than one parameter that related to its family; allowing us to correlate the data(such as "usage VS limit")
- Uses bulk functionalities of ES
- Creates daily indexes (allowing easy removal of old data, as Logstash and others do)
- Create aliases for the indexes

This is a big PR, which bring breaking changes(old schema not valid anymore). However- I have to admit that the old schema is totally unusable, and probably not many users use it if any(but me)

cc @huangyuqi @DirectXMan12 @piosz @Thermi

You can see the exported schema here: https://gist.github.com/AlmogBaku/8fe24fd9c61df5a72402a4d25992cb36

This change is

AlmogBaku · 2016-09-24T09:23:33Z

for some reason the tests takes too long, and it caused travis to fail..

AlmogBaku · 2016-10-07T13:29:16Z

@huangyuqi how can we push this forward?

AlmogBaku · 2016-10-10T09:56:56Z

signed the linux foundation cla

AlmogBaku · 2016-10-19T02:37:18Z

@piosz @huangyuqi ? how can we push this forward?

DirectXMan12 · 2016-10-19T15:22:38Z

@huangyuqi can you please do a review? I'm not particularly familiar with ElasticSearch

AlmogBaku · 2016-10-20T12:39:01Z

I fixed an issue where some of the points haven't committed due to not passing the threshold of the bulk-processor..

Now it's flush on one of the following scenarios:

every 10s
2MB of data
= 1000 requests
end of the processing(end of ExportData or ExportEvents)

piosz · 2016-10-24T11:34:56Z

@AlmogBaku thanks for your contribution. I'm fine with this PR, but I'm not familiarized with ES. We need someone to review the code. @commixon offered his help.

In the meantime could you please reorganize commits into the following pattern:

godeps changes
real changes

If you think you need more than 2 commits please explain why.

googlebot · 2016-10-24T12:51:19Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

googlebot · 2016-10-24T12:51:19Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

googlebot · 2016-10-24T13:00:05Z

CLAs look good, thanks!

googlebot · 2016-10-24T13:00:05Z

CLAs look good, thanks!

AlmogBaku · 2016-10-24T13:01:53Z

@piosz done.

AlmogBaku · 2016-10-24T16:10:09Z

Your test run exceeded 120 minutes.
Can someone try to fix this?

piosz · 2016-10-24T19:46:14Z

@k8s-bot ok to test

piosz · 2016-10-24T19:47:29Z

@AlmogBaku I don't know how to fix this. If Jenkins is green we can assume that tests pass.

piosz · 2016-10-26T07:44:57Z

@huangyuqi please create an issue and we can discuss it there. Thanks for offering the help

AlmogBaku · 2016-10-26T09:32:18Z

@piosz @huangyuqi this can be solved by using glide #1314 (or specifically see my review comment in #1335 )

andrejvanderzee · 2016-11-04T14:03:09Z

@piosz See #1363.

andrejvanderzee · 2016-11-07T09:41:47Z

Review status: 0 of 100 files reviewed at latest revision, 2 unresolved discussions.

common/elasticsearch/elasticsearch.go, line 37 at r4 (raw file):

  bulkProcessor *elastic.BulkProcessor
  base_index    string
}

Why underscore instead of camelcase?

common/elasticsearch/elasticsearch.go, line 44 at r4 (raw file):

func (esConfig ElasticSearchService) IndexAlias(date time.Time, typeName string) string {
  return date.Format(fmt.Sprintf("%s-%s-2006.01.02", esConfig.base_index, typeName))
}

This should be esSvc and not esConfig like below. Probably also better to use a pointer type as a receiver like below.

Comments from Reviewable

andrejvanderzee · 2016-11-07T09:51:13Z

Review status: 0 of 100 files reviewed at latest revision, 3 unresolved discussions.

common/elasticsearch/mapping.go, line 177 at r4 (raw file):

      "index": "not_analyzed"
    }
  }

Have you considered to disable _all (its enabled by default)? That will save disk space:

    "_default_": {
            "_all": {
                "enabled": false,
            },

Comments from Reviewable

andrejvanderzee · 2016-11-07T10:03:17Z

Review status: 0 of 100 files reviewed at latest revision, 3 unresolved discussions.

common/elasticsearch/mapping.go, line 177 at r4 (raw file):

Previously, andrejvanderzee wrote…

Have you considered to disable _all (its enabled by default)? That will save disk space:
    "_default_": {
            "_all": {
                "enabled": false,
            },

https://www.elastic.co/guide/en/elasticsearch/reference/current/mapping-all-field.html

Comments from Reviewable

andrejvanderzee · 2016-11-07T10:30:53Z

@AlmogBaku I have looked through the PR and added few comments.

AlmogBaku · 2016-11-14T09:49:36Z

Review status: 0 of 100 files reviewed at latest revision, 3 unresolved discussions.

common/elasticsearch/elasticsearch.go, line 37 at r4 (raw file):

Previously, andrejvanderzee wrote…

Why underscore instead of camelcase?

Done.

common/elasticsearch/elasticsearch.go, line 44 at r4 (raw file):

Previously, andrejvanderzee wrote…

This should be esSvc and not esConfig like below. Probably also better to use a pointer type as a receiver like below.

Done.

common/elasticsearch/mapping.go, line 177 at r4 (raw file):

Previously, andrejvanderzee wrote…

https://www.elastic.co/guide/en/elasticsearch/reference/current/mapping-all-field.html

Done.

Comments from Reviewable

events remapping fixes review comments flush data also in metrics

andrejvanderzee · 2016-11-14T10:08:19Z

Reviewed 5 of 9 files at r1, 90 of 91 files at r2, 3 of 4 files at r3, 2 of 2 files at r4, 4 of 4 files at r5.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable

AlmogBaku · 2016-11-14T12:42:17Z

@k8s-bot test this

k8s-ci-robot · 2016-11-14T12:42:18Z

@AlmogBaku: you can't request testing unless you are a kubernetes member.

In response to [this comment](https://github.com//pull/1313#issuecomment-260325392):

@k8s-bot test this

If you have questions or suggestions related to this bot's behavior, please file an issue against the kubernetes/test-infra repository.

andyxning · 2016-11-14T15:08:59Z

@AlmogBaku can you guys please have a look at #1379 #1374. I also make some RP based on the current master branch. BTW, creating index daily is really a wonderful thing as i also searched for how to make ES store time-limited events. :)

andyxning · 2016-11-20T07:43:38Z

Can we just split the new features into small ones and small PRs. In this way maybe we can push this forward faster. :)

AlmogBaku · 2016-11-20T13:30:31Z

@piosz, few people already review the code and that it functioning(see above).. can we merge this?

andyxning · 2016-11-20T13:57:14Z

@AlmogBaku Can you also please have a look at #1380 also. Ignore the build failure status, it seems that Heapster build will burst above the memory limit Travis CI container set and the build will be OOM killed. So, in #1379 i have updated the Travis CI configuration to use a VM which according to Travis CI has about 7.5GB memory except longer build start time. :)

piosz · 2016-11-23T13:18:02Z

I'm ok with the change. Can one of the reviewers confirm that it's ready to merge by writing LGTM?

piosz · 2016-11-23T13:18:11Z

@k8s-bot test this

andrejvanderzee · 2016-11-23T13:41:39Z

LGTM

k8s-ci-robot · 2016-11-23T14:24:10Z

Jenkins GCE e2e failed for commit d8894f3. Full PR test history.

The magic incantation to run this job again is @k8s-bot test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

piosz · 2016-11-23T18:06:00Z

Jenkins failure unrelated

googlebot added the cla: yes label Sep 23, 2016

huangyuqi self-assigned this Sep 24, 2016

AlmogBaku mentioned this pull request Sep 26, 2016

Production Quality Deployment coreos/coreos-kubernetes#340

Closed

18 tasks

AlmogBaku mentioned this pull request Oct 18, 2016

ElasticSearch | time-based index #1310

Closed

AlmogBaku force-pushed the feature/elasticsearch_mapping branch from a76afd1 to 56e4bc7 Compare October 24, 2016 12:51

googlebot added cla: no and removed cla: yes labels Oct 24, 2016

AlmogBaku force-pushed the feature/elasticsearch_mapping branch from 56e4bc7 to b7e84f2 Compare October 24, 2016 13:00

googlebot added cla: yes and removed cla: no labels Oct 24, 2016

AlmogBaku force-pushed the feature/elasticsearch_mapping branch 2 times, most recently from cbe2f6c to ffa3587 Compare October 24, 2016 13:20

AlmogBaku mentioned this pull request Oct 26, 2016

Elasticsearch sink not working #1211

Closed

andrejvanderzee mentioned this pull request Nov 4, 2016

Occasional negative Metrics.cpu/usage_rate.value #1363

Closed

AlmogBaku added 2 commits November 14, 2016 12:00

change the mapping to the new flat structure

4b3efc1

events remapping fixes review comments flush data also in metrics

update elastic dependency version

d8894f3

AlmogBaku force-pushed the feature/elasticsearch_mapping branch from 43a3915 to d8894f3 Compare November 14, 2016 10:01

AlmogBaku mentioned this pull request Nov 20, 2016

[ES] enhance es mapping for event #1379

Closed

piosz assigned ghost and unassigned huangyuqi Nov 23, 2016

piosz added the lgtm Indicates that a PR is ready to be merged. label Nov 23, 2016

piosz merged commit 21efd93 into kubernetes-retired:master Nov 23, 2016

AlmogBaku mentioned this pull request Nov 24, 2016

ES Sink: Don't marshal EventValue as String #1382

Closed

AlmogBaku mentioned this pull request Jun 8, 2017

ElasticSearch 5 support #1607

Merged

rikatz mentioned this pull request Jun 26, 2017

ElasticSearch sink - Improve mappings, timestamps, index separations, etc #1701

Closed

6 tasks

Feature/elasticsearch new mapping #1313

Feature/elasticsearch new mapping #1313

Conversation

AlmogBaku commented Sep 23, 2016 • edited Loading

AlmogBaku commented Sep 24, 2016

AlmogBaku commented Oct 7, 2016

AlmogBaku commented Oct 10, 2016

AlmogBaku commented Oct 19, 2016

DirectXMan12 commented Oct 19, 2016

AlmogBaku commented Oct 20, 2016

piosz commented Oct 24, 2016 • edited Loading

googlebot commented Oct 24, 2016

googlebot commented Oct 24, 2016

googlebot commented Oct 24, 2016

googlebot commented Oct 24, 2016

AlmogBaku commented Oct 24, 2016

AlmogBaku commented Oct 24, 2016

piosz commented Oct 24, 2016

piosz commented Oct 24, 2016

piosz commented Oct 26, 2016

AlmogBaku commented Oct 26, 2016

andrejvanderzee commented Nov 4, 2016

andrejvanderzee commented Nov 7, 2016

andrejvanderzee commented Nov 7, 2016

andrejvanderzee commented Nov 7, 2016

andrejvanderzee commented Nov 7, 2016

AlmogBaku commented Nov 14, 2016

andrejvanderzee commented Nov 14, 2016

AlmogBaku commented Nov 14, 2016

k8s-ci-robot commented Nov 14, 2016

andyxning commented Nov 14, 2016

andyxning commented Nov 20, 2016 • edited Loading

AlmogBaku commented Nov 20, 2016

andyxning commented Nov 20, 2016 • edited Loading

piosz commented Nov 23, 2016

piosz commented Nov 23, 2016

andrejvanderzee commented Nov 23, 2016

k8s-ci-robot commented Nov 23, 2016

piosz commented Nov 23, 2016

AlmogBaku commented Sep 23, 2016 •

edited

Loading

piosz commented Oct 24, 2016 •

edited

Loading

andyxning commented Nov 20, 2016 •

edited

Loading

andyxning commented Nov 20, 2016 •

edited

Loading