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

Migrating traefik module #763

Merged
merged 25 commits into from
Mar 11, 2021
Merged

Conversation

ycombinator
Copy link
Contributor

@ycombinator ycombinator commented Mar 5, 2021

What does this PR do?

Migrates the traefik module from Beats to Integrations.

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.

Screenshots

kibana-traefik

Related issues

@elasticmachine
Copy link

elasticmachine commented Mar 5, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #763 updated

  • Start Time: 2021-03-10T22:43:49.442+0000

  • Duration: 51 min 14 sec

  • Commit: c2a87a3

Test stats 🧪

Test Results
Failed 0
Passed 16
Skipped 0
Total 16

Trends 🧪

Image of Build Times

Image of Tests

@andresrc andresrc added the Team:Integrations Label for the Integrations team label Mar 6, 2021
@ycombinator ycombinator mentioned this pull request Mar 9, 2021
17 tasks
@ycombinator ycombinator marked this pull request as ready for review March 9, 2021 01:57
@elasticmachine
Copy link

Pinging @elastic/integrations (Team:Integrations)

@@ -0,0 +1,23 @@
- name: traefik.health
type: group
release: ga
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm... GA?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm guessing you want me to change it to experimental so it matches up with the release in the package-level manifest.yml?

BTW, this is what was generated by the migration script. Should we update it to generate release: experimental?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it's because originally integrations were supposed to be 1:1 modules. If a module was released GA, so the integration too. You can open an issue, but I'm not sure about it's priority (rather low, it's easy to adjust after migration).

Copy link
Contributor Author

@ycombinator ycombinator Mar 10, 2021

Choose a reason for hiding this comment

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

I just realized that this release: ga is in the fields.yml, not the manifest.yml. I'm fixing it to be release: experimental but I'm seeing some other packages (e.g. nats, docker) where the release is set to ga in their fields.yml but the package manifest.yml has release: experimental or release: beta.

Copy link
Contributor Author

@ycombinator ycombinator Mar 10, 2021

Choose a reason for hiding this comment

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

Actually, I ended up just removing the release field from this fields.yml file (c857182). I checked the spec for field definition files and that field is not even mentioned in there. So I'm not even sure of it's purpose or if we need it. 🤷

packages/traefik/data_stream/health/sample_event.json Outdated Show resolved Hide resolved
packages/traefik/manifest.yml Show resolved Hide resolved
packages/traefik/manifest.yml Outdated Show resolved Hide resolved

## Compatibility

The Traefik datasets were tested with Traefik 1.6.
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose that you didn't check compatibility with a more recent version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, not yet. In this PR I'm just focussing on the basic migration. I will make a follow up PR to test with a more recent version and then make the necessary updates to the package.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ignore my previous comment. I updated the version of Traefik in this PR to 1.7.

There is a 2.x version series as well but I think we should leave that for a follow up PR as it might contain breaking changes. WDYT?

@mtojek
Copy link
Contributor

mtojek commented Mar 10, 2021

jenkins run the tests please

@mtojek mtojek self-requested a review March 10, 2021 08:51
Copy link
Contributor

@mtojek mtojek left a comment

Choose a reason for hiding this comment

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

I think the only concern I have is to take a fresh screenshot. You can also put one in the issue description. @sorantis used to give suggestions what should be improved there.

@ycombinator ycombinator merged commit 2d0e5f7 into elastic:master Mar 11, 2021
@ycombinator ycombinator deleted the migrate-traefik branch March 11, 2021 15:28
eyalkraft pushed a commit to build-security/integrations that referenced this pull request Mar 30, 2022
* Migrating traefik module

* Formatting package files

* Removing invalid path field

* Adding categories

* Adding changelog file

* Formatting tweaks

* Adding pipeline test files

* Adding YAML header

* Adding system tests

* Reformatting

* Renaming pipeline test case files

* Fixing pipeline tests

* Adding ECS fields for health data stream

* Adding README template file

* Adding sample event for health data set

* Adding system test for access data stream

* Reformatting

* Adding README

* Removing host field from sample event files

* Fixing kibana compatibility version

* Testing with version 1.7

* Updating README

* Remove release field from fields.yml

* Remove temp field

* Updating screenshot
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Integrations Label for the Integrations team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants