Skip to content

[GitLab] Update params mapping for nested object arrays #12286

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 0 commits into from
Jan 14, 2025

Conversation

tehbooom
Copy link
Member

@tehbooom tehbooom commented Jan 9, 2025

Proposed commit message

For the GitLab API datastream the params field is an array of objects that are dynamic. The fields were broken into key and value fields but broke when the value was another object.

This PR breaks each key value into their own field and adds field mappings for those fields.

For example this

"params": [
    {
        "key": "private_token",
        "value": "[FILTERED]"
    }
]

is now this with correct field mappings.

"params": [
    {
        "private_token": "[FILTERED]"
    }
]

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

  • [ ]

How to test this PR locally

Related issues

Screenshots

@tehbooom tehbooom added bugfix Pull request that fixes a bug issue Integration:gitlab GitLab labels Jan 9, 2025
@tehbooom tehbooom requested a review from a team as a code owner January 9, 2025 13:51
@tehbooom tehbooom added the Team:Security-Service Integrations Security Service Integrations team [elastic/security-service-integrations] label Jan 9, 2025
@elasticmachine
Copy link

Pinging @elastic/security-service-integrations (Team:Security-Service Integrations)

@elastic-vault-github-plugin-prod

🚀 Benchmarks report

To see the full report comment with /test benchmark fullreport

Copy link
Contributor

@chemamartinez chemamartinez left a comment

Choose a reason for hiding this comment

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

I have noticed that the production data stream already handles this issue with a different approach. In that case, value is renamed to value_json and mapped as an object in case it is not a string.

I do prefer the solution given in this PR, but it makes me wonder a couple of things:

  • Shall we unify the criteria here? that would make the integration more consistent.
  • If we follow this approach, we could cause a breaking change in the other data stream by stopping using the value_json field.

@tehbooom
Copy link
Member Author

I have noticed that the production data stream already handles this issue with a different approach. In that case, value is renamed to value_json and mapped as an object in case it is not a string.

I do prefer the solution given in this PR, but it makes me wonder a couple of things:

  • Shall we unify the criteria here? that would make the integration more consistent.
  • If we follow this approach, we could cause a breaking change in the other data stream by stopping using the value_json field.

I do prefer the solution in this PR as I think it presents the fields in a cleaner way and easier to understand. However, that does require an adjustment to the other datastream to ensure they line up correctly.

@chemamartinez
Copy link
Contributor

I have noticed that the production data stream already handles this issue with a different approach. In that case, value is renamed to value_json and mapped as an object in case it is not a string.
I do prefer the solution given in this PR, but it makes me wonder a couple of things:

  • Shall we unify the criteria here? that would make the integration more consistent.
  • If we follow this approach, we could cause a breaking change in the other data stream by stopping using the value_json field.

I do prefer the solution in this PR as I think it presents the fields in a cleaner way and easier to understand. However, that does require an adjustment to the other datastream to ensure they line up correctly.

Yes, me too. I have checked that these value_json params are not being used in any dashboards, and in this particular case I think it is unlikely that anyone is using these fields to create custom assets. I'd go with the change for both data streams.

Copy link

@elasticmachine
Copy link

💚 Build Succeeded

History

  • 💚 Build #20359 succeeded bd54f962675ebb64ed4ec4dd76f5e44360321eef
  • 💔 Build #20357 failed 84acd66fba5e6a3e7413c7ac67f8e0a78a685102
  • 💚 Build #20231 succeeded ace0fb45ab4d0687d2dbe66a5d0f1fa85fff3cfa

Copy link
Contributor

@chemamartinez chemamartinez left a comment

Choose a reason for hiding this comment

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

LGTM!

@tehbooom tehbooom merged commit 8b8fe3f into elastic:main Jan 14, 2025
5 checks passed
@elastic-vault-github-plugin-prod

Package gitlab - 2.0.0 containing this change is available at https://epr.elastic.co/package/gitlab/2.0.0/

harnish-crest-data pushed a commit to chavdaharnish/integrations that referenced this pull request Feb 4, 2025
…roduction data streams (elastic#12286)

* fix: Breakout kv fields and add mappings for api datastream

* chore: Update PR number

* feat: Align production data stream to match kv pairs in api data stream

* docs: Update README with new fields

* chore: Update version
harnish-crest-data pushed a commit to chavdaharnish/integrations that referenced this pull request Feb 5, 2025
…roduction data streams (elastic#12286)

* fix: Breakout kv fields and add mappings for api datastream

* chore: Update PR number

* feat: Align production data stream to match kv pairs in api data stream

* docs: Update README with new fields

* chore: Update version
@tehbooom tehbooom deleted the gitlab-11941 branch February 6, 2025 21:53
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:gitlab GitLab Team:Security-Service Integrations Security Service Integrations team [elastic/security-service-integrations]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[GitLab]: gitlab.api logs with nested key-value pairs fail to index
3 participants