Skip to content

AWS ELB add support for ALPN policy details in NLB logs #11590

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 5 commits into from
Nov 6, 2024

Conversation

agithomas
Copy link
Contributor

@agithomas agithomas commented Oct 30, 2024

  • Bug

Proposed commit message

Add support for extracting ALPN policy related details from NLB access log data.

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.
  • I have verified that any added dashboard complies with Kibana's Dashboard good practices

Author's Checklist

  • Update the new pattern in the pipeline test files and generate new expected log data
  • Package upgrade testing

How to test this PR locally

elastic-package build && elastic-package stack up -v -d --services package-registry

Related issues

Screenshots

image

@andrewkroh andrewkroh added bugfix Pull request that fixes a bug issue Integration:aws AWS labels Oct 30, 2024
@elastic-vault-github-plugin-prod
Copy link

elastic-vault-github-plugin-prod bot commented Oct 30, 2024

🚀 Benchmarks report

To see the full report comment with /test benchmark fullreport

@agithomas agithomas marked this pull request as ready for review October 30, 2024 15:06
@agithomas agithomas requested review from a team as code owners October 30, 2024 15:06
@andrewkroh andrewkroh added the Team:Obs-InfraObs Observability Infrastructure Monitoring team [elastic/obs-infraobs-integrations] label Oct 30, 2024
Copy link
Member

@shmsr shmsr left a comment

Choose a reason for hiding this comment

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

Left one comment. Rest looks good!

Copy link
Contributor

@lucian-ioan lucian-ioan left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@kcreddy kcreddy left a comment

Choose a reason for hiding this comment

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

Adding code owner approval.

@agithomas
Copy link
Contributor Author

@elastic/obs-ds-hosted-services , could you please review the PR?

Copy link
Contributor

@zmoog zmoog left a comment

Choose a reason for hiding this comment

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

LGTM, I left a question about alpn_client_preference_list.

Comment on lines +132 to +133
description: >
The value of the application_layer_protocol_negotiation extension in the client hello message. This value is URL-encoded.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did we keep the alpn_client_preference_list in its source form and not parse it as a list? Do we expect users to need it in its pristine form?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this is needed, we can do something like this. Keeping it for further reference / enhancement

  - script:
      lang: painless
      source: >-
        if (ctx?.aws?.elb?.alpn_client_preference_list != null && ctx?.aws?.elb?.alpn_client_preference_list != '-' && !ctx?.aws?.elb?.alpn_client_preference_list.trim().isEmpty()) {
          String cleanedList = ctx.aws.elb.alpn_client_preference_list;
          // Use regex to split by comma
          String[] protocols = /,/.split(cleanedList);

          ArrayList filteredProtocols = new ArrayList();
          for (String protocol : protocols) {
            def protocol_temp = /[\\"]/.matcher(protocol).replaceAll("");
            if (!protocol_temp.trim().isEmpty()) {
                filteredProtocols.add(protocol_temp.trim());
            }
          }
          ctx.aws.elb.alpn_client_preference_list = filteredProtocols;
        } else {
            // Set to an empty array if the conditions are not met
            ctx.aws.elb.alpn_client_preference_list = [];
        }

@agithomas
Copy link
Contributor Author

/test

…e/default.yml

Co-authored-by: kaiyan-sheng <kaiyan.sheng@elastic.co>
@elasticmachine
Copy link

💚 Build Succeeded

History

cc @agithomas

Copy link

@agithomas agithomas merged commit 4f2be28 into elastic:main Nov 6, 2024
5 checks passed
@elastic-vault-github-plugin-prod

Package aws - 2.31.2 containing this change is available at https://epr.elastic.co/search?package=aws

harnish-crest-data pushed a commit to chavdaharnish/integrations that referenced this pull request Feb 4, 2025
* AWS ELB add support for ALPN policy details in NLB logs
harnish-crest-data pushed a commit to chavdaharnish/integrations that referenced this pull request Feb 5, 2025
* AWS ELB add support for ALPN policy details in NLB logs
@agithomas agithomas deleted the issue-11587 branch February 12, 2025 04:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Pull request that fixes a bug issue Integration:aws AWS Team:Obs-InfraObs Observability Infrastructure Monitoring team [elastic/obs-infraobs-integrations]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[AWS][ELB LOGS] [NLB Access logs] Support for ALPN Policy
9 participants