Skip to content

Conversation

@Wiston999
Copy link

@Wiston999 Wiston999 commented Jul 18, 2023

Add target_index variable for rendering ES index name using record accessor syntax. This allows to define ES index name based on record accessor, enabling to use record field values as destination ES index.
It differs from logstash_prefix_key as no time component is added to index name, so ES data streams can be used as target index easily.

Tests output and other information at https://gist.github.com/Wiston999/c7d3ff5389f64fdf1113146090ab1aa1.

Addresses #2514


Enter [N/A] in the box, if an item is not applicable to your change.

Testing
Before we can approve your change; please submit the following in a comment:

  • Attached Valgrind output that shows no leaks or memory corruption was found (here)

If this is a change to packaging of containers or native binaries then please confirm it works for all targets.

  • [N/A] Run local packaging test showing all targets (including any new ones) build.
  • [N/A] Set ok-package-test label to test for all targets (requires maintainer to do).

Documentation

  • Documentation required for this feature

fluent/fluent-bit-docs#1163

Backporting

  • [N/A] Backport to latest stable release.

Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.

@patrick-stephens
Copy link
Collaborator

I think for Opensearch, Index was just updated to support RA syntax - would it be better to do that for ES as well rather than a new key?

@Wiston999 Wiston999 temporarily deployed to pr July 18, 2023 13:42 — with GitHub Actions Inactive
@Wiston999 Wiston999 temporarily deployed to pr July 18, 2023 13:42 — with GitHub Actions Inactive
@Wiston999
Copy link
Author

I think for Opensearch, Index was just updated to support RA syntax - would it be better to do that for ES as well rather than a new key?

I considered that option too. I'm open to change to RA on Index and adding a new Index_default or whatever variable name for a fallback index name in case RA doesn't render any value.

Just confirm me if you prefer that option and I'll make the changes.

@Wiston999
Copy link
Author

I've updated the PR to use the same approach as out_opensearch plugin, it is, detect if Index variable has $ and treat it like a RA expression.

Updated gist .

@Wiston999 Wiston999 temporarily deployed to pr July 20, 2023 09:09 — with GitHub Actions Inactive
@Wiston999 Wiston999 temporarily deployed to pr July 20, 2023 09:09 — with GitHub Actions Inactive
@Wiston999 Wiston999 temporarily deployed to pr July 20, 2023 09:09 — with GitHub Actions Inactive
@Wiston999 Wiston999 temporarily deployed to pr July 20, 2023 09:40 — with GitHub Actions Inactive
@DandyDeveloper
Copy link

Anybody looking at this? I'm also impacted and seems as though opensearch as an alternative broke with the API changes in recent release, so I can't use that as a bandaid.

👍

@braydonk
Copy link
Contributor

What happened with the commits on this PR? Whatever happened last week, where it added a bunch of commits from master, should probably be resolved. Only the 3 out_es commits that are actually part of this PR should be here.

@Wiston999 Wiston999 force-pushed the out-es-target-index-key branch from fd14fe2 to 8a6a409 Compare August 16, 2023 08:29
@Wiston999
Copy link
Author

What happened with the commits on this PR? Whatever happened last week, where it added a bunch of commits from master, should probably be resolved. Only the 3 out_es commits that are actually part of this PR should be here.

I've just fixed the mess with commits from master. I'm sorry for the noise introduced in the PR and for the inconveniences to other committers.

@Wiston999 Wiston999 temporarily deployed to pr August 16, 2023 15:07 — with GitHub Actions Inactive
@Wiston999 Wiston999 temporarily deployed to pr August 16, 2023 15:07 — with GitHub Actions Inactive
@DandyDeveloper
Copy link

Any ideas on when this might get merged in?

@DandyDeveloper
Copy link

@edsiper @leonardo-albertovich Can we get some eyes on this please?

@ajax-bychenok-y
Copy link

ajax-bychenok-y commented Oct 9, 2023

@DandyDeveloper I came up with next idea, maybe it will be useful for you. It's clusteroutput for K8s fluentbit-operator resource but I think it's quite easily can be transformed to just daemon config (check spec section):

apiVersion: fluentbit.fluent.io/v1alpha2
kind: ClusterOutput
metadata:
  annotations:
    meta.helm.sh/release-name: fluent-operator
    meta.helm.sh/release-namespace: fluent
  labels:
    app.kubernetes.io/managed-by: Terraform
    fluentbit.fluent.io/component: logging
    fluentbit.fluent.io/enabled: "true"
  name: es
spec:
  es:
    bufferSize: 50MB
    generateID: true
    host: ${output_es_host}
    # That's a hack for not creating datastreams with date suffix
    logstashDateFormat: eks
    logstashFormat: true
    # In this index go all records w/o proper label 
    logstashPrefix: company-unknown
    logstashPrefixKey: kubernetes['labels']['app.kubernetes.io/name']
    port: 9200
    replaceDots: false
    suppressTypeName: "On"
    timeKey: '@timestamp'
  matchRegex: (?:kube|service)\.(.*)

So fluentbit sends everything in kubernetes['labels']['app.kubernetes.io/name'] datastream name (index template with datastream creating option needs to be previously turned on) and adds eks suffix in the end. Yes, it's quite hacky.

@DandyDeveloper
Copy link

@ajax-bychenok-y Thanks for the suggestion! This could work in theory actually, I'll have a crack. But, I'm a little shocked this PR has been pending for so long... It's a pretty simple addition.

Add target_index variable for rendering ES index name using record
accessor syntax without extra bytes (i.e.: no time component on index
name).

Signed-off-by: Victor Cabezas <[email protected]>
@Wiston999
Copy link
Author

Hi @edsiper or any other maintainer. Are these changes being considered to be merged?
If so, is there any pending action from my side to be taken that is preventing the PR to be merged?

We would like to start upgrading our ES clusters to v8 and as @DandyDeveloper pointed out these changes are needed to have index RA behavior on ES v8 clusters.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 8, 2024

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the Stale label Mar 8, 2024
@DandyDeveloper
Copy link

Anyone alive

@2nfree
Copy link

2nfree commented Jun 28, 2024

Is anyone else reviewing or keeping track of this commit?

@cw-Guo
Copy link
Contributor

cw-Guo commented Jul 16, 2024

Hi @cosmo0920 @edsiper , is there anything else needed to do in this pr before it can be merged?

(char *) tag, tag_len,
map, NULL);
if (v) {
len = flb_sds_len(v);
Copy link
Contributor

Choose a reason for hiding this comment

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

either dynamically allocating the index so it is not truncated or returning a warning or error might be better idea. The limit should also be checked against the actual size of index_value, (sizeof()-1), instead of a hardcoded value.

@github-actions
Copy link
Contributor

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the Stale label Nov 27, 2024
@github-actions github-actions bot removed the Stale label Mar 22, 2025
@github-actions
Copy link
Contributor

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot added the Stale label Aug 28, 2025
@eschabell
Copy link

@Wiston999 would you mind addressing the conflicts and @patrick-stephens can you look at this for a review?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants