Skip to content
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

fix(chart/opensearch-dashboards): mapper_parsing_exception - tried to parse field [app] as object, but found a concrete value #629

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

ainthapanya-sqc
Copy link

@ainthapanya-sqc ainthapanya-sqc commented Dec 5, 2024

Description

This pull request addresses an issue where using the app label causes conflicts with object mapping in OpenSearch Dashboards. To resolve this, the app label has been replaced with the app.kubernetes.io/name label across the codebase.

Issues Resolved

Check List

  • Commits are signed per the DCO using --signoff

For any changes to files within Helm chart directories:

  • Helm chart version bumped
  • Helm chart CHANGELOG.md updated to reflect change

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@DandyDeveloper
Copy link
Collaborator

@ainthapanya-sqc Where's the original chart you used for Fluent Bit to reproduce this? There's lots that float around,

Also, that mapping parser exceptions should be coming from Opensearch API downstream to Fluent when it attempts to bulk push the docs.

Are you sure your mapping on the index aren't misconfigured? As the implication is that an index mapping is wrong on your index (Object, not String/Keyword for example.)

@ainthapanya-sqc
Copy link
Author

ainthapanya-sqc commented Jan 6, 2025

@DandyDeveloper I had used Fluent Bit Helm chart 0.48.3.

Are you sure your mapping on the index aren't misconfigured? As the implication is that an index mapping is wrong on your index (Object, not String/Keyword for example.)

Pretty sure, the field kubernetes.labels.app should be of type object
However this helm chart, seems to have the kubernetes.labels.app field as a string.

There are other apps/deployments which are using appropriate labels and is working out of the box.

As a workaround, I needed to manually modify kubernetes.labels.app from string to an object in this helm chart.

Additionally, this may help with some alignment with the common/recommended labels.
https://kubernetes.io/docs/concepts/overview/working-with-objects/common-labels/#labels

image

@DandyDeveloper
Copy link
Collaborator

@ainthapanya-sqc Okay, sorry I see the problem you're having. It's because in our manifests, we're using app as a label instead of the commonly found object of app.kubernetes.io/name etc. The fluent-bit parser is presumably grabbing logs throughout your cluster and funneling into the same index.

We should actually use the existing _helper template:

{{/*
Selector labels
*/}}
{{- define "opensearch-dashboards.selectorLabels" -}}
app.kubernetes.io/name: {{ include "opensearch-dashboards.name" . }}
app.kubernetes.io/instance: {{ .Release.Name }}
{{- end }}

Can you update to use this and make sure the DCO stuff is done? Then I'll approve.

@DandyDeveloper
Copy link
Collaborator

@ainthapanya-sqc Need to sort out the DCO related checks, but the code itself LGTM!

@ainthapanya-sqc ainthapanya-sqc force-pushed the update-labels branch 3 times, most recently from 2c64ff2 to db792cc Compare January 9, 2025 00:19
LemonDouble and others added 8 commits January 9, 2025 11:22
… plugin conflicts (opensearch-project#618)

* Remove opensearch.yml config in Values.yaml to avoid security plugin conflicts

Signed-off-by: LemonDouble <[email protected]>

* bump version

Signed-off-by: LemonDouble <[email protected]>

* add space for lint

Signed-off-by: LemonDouble <[email protected]>

* fix : Update CHANGELOG.md

Co-authored-by: Craig Perkins <[email protected]>
Signed-off-by: LemonDouble <[email protected]>

* fix : comment ci's opensearch.yml to fix ci pipeline

Signed-off-by: LemonDouble <[email protected]>

* fix : update CHANGELOG.md

Co-authored-by: Craig Perkins <[email protected]>
Signed-off-by: LemonDouble <[email protected]>

* fix : comment only security plugin configs

Signed-off-by: LemonDouble <[email protected]>

---------

Signed-off-by: LemonDouble <[email protected]>
Signed-off-by: LemonDouble <[email protected]>
Co-authored-by: Craig Perkins <[email protected]>
Signed-off-by: ainthapanya-sqc <[email protected]>
Signed-off-by: ainthapanya-sqc <[email protected]>
Signed-off-by: ainthapanya-sqc <[email protected]>
Signed-off-by: ainthapanya-sqc <[email protected]>
Signed-off-by: ainthapanya-sqc <[email protected]>
Signed-off-by: ainthapanya-sqc <[email protected]>
Signed-off-by: ainthapanya-sqc <[email protected]>
Signed-off-by: ainthapanya-sqc <[email protected]>
Signed-off-by: ainthapanya-sqc <[email protected]>
Signed-off-by: ainthapanya-sqc <[email protected]>
Signed-off-by: ainthapanya-sqc <[email protected]>
@ainthapanya-sqc
Copy link
Author

@DandyDeveloper may need your help in approving the workflow.

@DandyDeveloper
Copy link
Collaborator

@ainthapanya-sqc Triggered them. I'll keep an eye out.

@DandyDeveloper
Copy link
Collaborator

@peterzhuamazon We're good to merge this whenever you're happy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 👀 In Review
Development

Successfully merging this pull request may close these issues.

3 participants