Skip to content

Add runtime field flag in documentation files #2415

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 11 commits into from
Feb 19, 2025

Conversation

mrodm
Copy link
Contributor

@mrodm mrodm commented Feb 17, 2025

Closes #1229

Description

This PR updates the generation of fields documentation to include the runtime flag in the corresponding field definitions.

Example of the docs if runtime flag is added in the same column as the type:

| day_of_week | Day of week. | keyword(runtime) |  |  |
| labels.\* |  | keyword(runtime) |  |  |
| package_registry.day_of_week | Other day of week. | keyword(runtime) |  |  |
| package_registry.day_of_week2 | Day of week 2. | keyword(runtime) |  |  |
| package_registry.labels.version | Elastic Package Registry version. | keyword(runtime) |  |  |

How to test this PR locally

Go to a package and add some runtime field definitions:

- name: labels.*
  type: keyword
  runtime: true
- name: labels.a
  description: "My label a"
  type: text

- name: package_registry.day_of_week
  description: "Other day of week."
  type: keyword
  runtime: "emit(doc['@timestamp'].value.dayOfWeekEnum.getDisplayName(TextStyle.FULL, Locale.ENGLISH))"

- name: package_registry
  type: group
  fields:
    - name: day_of_week2
      description: Day of week 2.
      type: keyword
      runtime: "emit(doc['@timestamp'].value.dayOfWeekEnum.getDisplayName(TextStyle.FULL, Locale.ENGLISH))"

- name: day_of_week
  type: keyword
  description: Day of week.
  runtime: "emit(doc['@timestamp'].value.dayOfWeekEnum.getDisplayName(TextStyle.FULL, Locale.ENGLISH))"

Build your package with elastic-package and check the README docs built:

cd /path/to/package

elastic-package build -v

# check changes in doc files

Update how string is built by using fmt.Fprintf. Related to the static
check QF1012 https://staticcheck.dev/docs/checks/#QF1012
@mrodm mrodm self-assigned this Feb 17, 2025
@@ -44,82 +43,6 @@ type ReusableConfig struct {
TopLevel bool `yaml:"top_level"`
}

func (orig *FieldDefinition) Update(fd FieldDefinition) {
Copy link
Contributor Author

@mrodm mrodm Feb 17, 2025

Choose a reason for hiding this comment

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

I didn't find any usages of this Update function in the elastic-package code base.

It looks like that it was removed in this PR #1335:
https://github.com/elastic/elastic-package/pull/1335/files#diff-77cc155ee037ac620b1e188302f60fdf736637a25e5021e76dbad83594edd09cL133

Copy link
Member

Choose a reason for hiding this comment

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

Interesting, this used to be neccesary 🤔 But good if we don't need it anymore.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At least, that function is not called anymore...

@mrodm
Copy link
Contributor Author

mrodm commented Feb 18, 2025

/test

2 similar comments
@mrodm
Copy link
Contributor Author

mrodm commented Feb 18, 2025

/test

@mrodm
Copy link
Contributor Author

mrodm commented Feb 18, 2025

/test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed test (file) related to the Update function.

@mrodm
Copy link
Contributor Author

mrodm commented Feb 18, 2025

In this PR, it is implemented that the runtime flag is added next to the type of the field:

| day_of_week | Day of week. | keyword (runtime) |  |  |
| labels.\* |  | keyword (runtime) |  |  |
| package_registry.day_of_week | Other day of week. | keyword (runtime) |  |  |
| package_registry.day_of_week2 | Day of week 2. | keyword (runtime) |  |  |
| package_registry.labels.version | Elastic Package Registry version. | keyword (runtime) |  |  |

Another option would be to set the runtime flag in the first column (field name). It could be like this:

| day_of_week (runtime) | Day of week. | keyword |  |  |
| labels.\* (runtime) |  | keyword|  |  |
| package_registry.day_of_week (runtime) | Other day of week. | keyword |  |  |
| package_registry.day_of_week2 (runtime) | Day of week 2. | keyword |  |  |
| package_registry.labels.version (runtime) | Elastic Package Registry version. | keyword |  |  |

Which of the two ways do you think shows better the runtime fields to the user?

@mrodm mrodm changed the title Runtime field docs - WIP Add runtime field flag in documentation files Feb 18, 2025
@mrodm mrodm marked this pull request as ready for review February 18, 2025 15:02
@mrodm mrodm requested a review from a team February 18, 2025 15:02
@jsoriano
Copy link
Member

Which of the two ways do you think shows better the runtime fields to the user?

I think it would be better in the type? 🤔 But not a strong opinion, as you prefer.

Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

Looks good, but can this break current integrations builds? Or there is no package using runtime?

@@ -44,82 +43,6 @@ type ReusableConfig struct {
TopLevel bool `yaml:"top_level"`
}

func (orig *FieldDefinition) Update(fd FieldDefinition) {
Copy link
Member

Choose a reason for hiding this comment

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

Interesting, this used to be neccesary 🤔 But good if we don't need it anymore.

@mrodm
Copy link
Contributor Author

mrodm commented Feb 18, 2025

Looks good, but can this break current integrations builds? Or there is no package using runtime?

Currently, there are no packages using runtime fields in the integrations repository:

 $ git grep "runtime:"  |grep "yml:" |grep "fields/"
 $

EDIT:

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @mrodm

@mrodm
Copy link
Contributor Author

mrodm commented Feb 18, 2025

test integrations

@elastic-vault-github-plugin-prod

Created or updated PR in integrations repository to test this version. Check elastic/integrations#12824

@mrodm
Copy link
Contributor Author

mrodm commented Feb 19, 2025

Packages failing in that PR are the expected ones related to validation based on mappings:
https://buildkite.com/elastic/integrations/builds/22461

@mrodm mrodm merged commit 78f5cd0 into elastic:main Feb 19, 2025
3 checks passed
@mrodm mrodm deleted the runtime_field_docs branch February 19, 2025 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for runtime fields
3 participants