Skip to content

Network community_id processor for ingest pipelines#66534

Merged
danhermann merged 4 commits intoelastic:masterfrom
danhermann:55685_community_id_processor
Jan 13, 2021
Merged

Network community_id processor for ingest pipelines#66534
danhermann merged 4 commits intoelastic:masterfrom
danhermann:55685_community_id_processor

Conversation

@danhermann
Copy link
Copy Markdown
Contributor

@danhermann danhermann commented Dec 17, 2020 •
edited
Loading

Adds a processor that computes the community_id for flow data according to the Community ID Specification.

For example:

POST _ingest/pipeline/_simulate?verbose
{
  "pipeline": {
    "processors": [
      {
        "community_id": {
        }
      }
    ]
  },
  "docs": [
    {
      "_source": {
        "source": {
          "ip": "128.232.110.120",
          "port": 34855
        },
        "destination": {
          "ip": "66.35.250.204",
          "port": 80
        },
        "network": {
          "transport": "TCP"
        }
      }
    }
  ]
}

populates the network.community_id field as below:

...
"_source" : {
  "destination" : {
    "port" : 80,
    "ip" : "66.35.250.204"
  },
  "source" : {
    "port" : 34855,
    "ip" : "128.232.110.120"
  },
  "network" : {
    "community_id" : "1:LQU9qZlK+B5F3KDmev6m5PMibrg=",
    "transport" : "TCP"
  }
}
...

Closes #55685

@danhermann danhermann added >feature :Distributed/Ingest Node Execution or management of Ingest Pipelines v8.0.0 v7.12.0 labels Dec 17, 2020
@elasticmachine elasticmachine added the Team:Data Management (obsolete) DO NOT USE. This team no longer exists. label Dec 17, 2020
@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented Dec 17, 2020

Pinging @elastic/es-core-features (Team:Core/Features)

@danhermann
Copy link
Copy Markdown
Contributor Author

danhermann commented Dec 17, 2020

@andrewkroh, I would appreciate a look at this to see if it meets the criteria for #55685. I did replicate all the unit tests for the Beats community_id processor to verify that this processor produces the same values, so I'm reasonably confident about it in that regard.

@danhermann
Copy link
Copy Markdown
Contributor Author

danhermann commented Dec 17, 2020

cc: @elastic/es-ui in case Kibana auto-complete needs to be updated with this new processor.

@alisonelizabeth alisonelizabeth mentioned this pull request Dec 17, 2020
Closed
@alisonelizabeth
Copy link
Copy Markdown
Contributor

alisonelizabeth commented Dec 17, 2020

Thanks @danhermann. I've opened elastic/kibana#86321 to track changes needed in Kibana. Can you also share the available options for this processor?

@danhermann
Copy link
Copy Markdown
Contributor Author

danhermann commented Dec 17, 2020

Thanks @danhermann. I've opened elastic/kibana#86321 to track changes needed in Kibana. Can you also share the available options for this processor?

Yes, I'll open a separate PR with the docs for this processor that will include all its available options. I'll tag you on that one when I open it.

andrewkroh
andrewkroh reviewed Dec 17, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

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

This looks great. Just a minor comment from me.

x-pack/plugin/ingest/src/main/java/org/elasticsearch/xpack/ingest/CommunityIdProcessor.java Outdated

Flow flow = new Flow();
try {
flow.source = InetAddress.getByName(sourceIpAddrString);
Copy link
Copy Markdown
Member

@andrewkroh andrewkroh Dec 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I'm worried that this could potentially perform network calls to lookup IP addresses from names.

Luckily we have a utility for this that's safe. Can you use that package here?

elasticsearch/server/src/main/java/org/elasticsearch/common/network/InetAddresses.java

Line 340 in feab123

public static InetAddress forString(String ipString) {

x-pack/plugin/ingest/src/main/java/org/elasticsearch/xpack/ingest/CommunityIdProcessor.java
return transportNumber;
}

public static Transport fromNumber(int transportNumber) {
Copy link
Copy Markdown
Member

@andrewkroh andrewkroh Dec 17, 2020

Choose a reason for hiding this comment

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

Not sure if this is a worth it, so feel free to ignore. To ensure that fromNumber stays up-to-date with all the enum transportNumbers it might be useful to have test that checks the all the Transport.values() work with fromNumber without exception.

@danhermann danhermann mentioned this pull request Dec 18, 2020
Merged
@danhermann
Copy link
Copy Markdown
Contributor Author

danhermann commented Dec 18, 2020

@alisonelizabeth, the processor's options are documented here: #66592

@danhermann
Copy link
Copy Markdown
Contributor Author

danhermann commented Dec 18, 2020

This looks great. Just a minor comment from me.

Thanks, @andrewkroh! I've made both changes that you suggested.

andrewkroh
andrewkroh approved these changes Dec 18, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

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

LGTM

@jakelandis jakelandis requested a review from martijnvg January 7, 2021 16:17
martijnvg
martijnvg approved these changes Jan 12, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

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

For processors added in ingest-common module there is a yaml test (Ingest common installed) that ensures that processors are correct wired up. Maybe we should have such a test for the xpack ingest module as well? Then we verify that uri_parts and community_id processors have correctly been wired up and can be used in pipelines.

Ingest integration LGTM.

x-pack/plugin/ingest/src/main/java/org/elasticsearch/xpack/ingest/CommunityIdProcessor.java Outdated
import static org.elasticsearch.ingest.ConfigurationUtils.newConfigurationException;
import static org.elasticsearch.ingest.ConfigurationUtils.readBooleanProperty;

public class CommunityIdProcessor extends AbstractProcessor {
Copy link
Copy Markdown
Member

@martijnvg martijnvg Jan 12, 2021

Choose a reason for hiding this comment

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

make class final?

@danhermann
Copy link
Copy Markdown
Contributor Author

danhermann commented Jan 13, 2021

@elasticmachine update branch

@danhermann
Copy link
Copy Markdown
Contributor Author

danhermann commented Jan 13, 2021

Thanks, @martijnvg! I'll open another PR with the module yaml test that you suggested.

@danhermann danhermann merged commit 1bb90a7 into elastic:master Jan 13, 2021
@danhermann danhermann deleted the 55685_community_id_processor branch January 13, 2021 15:31
danhermann added a commit to danhermann/elasticsearch that referenced this pull request Jan 14, 2021
@danhermann
Network community_id processor for ingest pipelines (elastic#66534)
0b9672a
@danhermann danhermann mentioned this pull request Jan 14, 2021
Merged
danhermann added a commit that referenced this pull request Jan 14, 2021
@danhermann
[7.x] Network community_id processor for ingest pipelines (#66534) (#…
21c3970 
…67518)
@cjcenizal cjcenizal mentioned this pull request Feb 11, 2021
Closed
@stevejgordon stevejgordon mentioned this pull request Feb 22, 2021
Closed
34 tasks
This was referenced Feb 23, 2021
Merged
Merged
This was referenced Mar 5, 2021
Merged
Merged
@joegallo joegallo mentioned this pull request Feb 7, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@martijnvg martijnvg martijnvg approved these changes

@andrewkroh andrewkroh andrewkroh approved these changes

Assignees

No one assigned

Labels

:Distributed/Ingest Node Execution or management of Ingest Pipelines >feature Team:Data Management (obsolete) DO NOT USE. This team no longer exists. v7.12.0 v8.0.0-alpha1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Ingest Node processor for Network Community ID

6 participants

@danhermann @elasticmachine @alisonelizabeth @martijnvg @andrewkroh @jakelandis