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

Oracle WebLogic: Initial skeleton along with 4 data streams structure #2910

Closed

Conversation

sunny-elastic
Copy link
Contributor

What does this PR do?

  • Generated the skeleton of Oracle WebLogic integration package.
  • Added skeleton for 4 data streams (Log, GC, JVM, JMS)

Analysis

We were able to find the following Logs and Metrics

  • Admin server logs
  • Managed server logs
  • Domain logs
  • HTTP access logs
  • JVM metrics
  • Garbage collection metrics
  • Work Managers metrics
  • JMS metrics
  • JDBC metrics

In the current scope, we are planning to implement the following Logs and Metrics, which will include 4 data streams/types in V1 as follows :

  1. log (Admin server logs, Managed server logs, Domain logs, HTTP access logs)
  2. gc (Garbage collection metrics)
  3. jvm (JVM metrics)
  4. jms (JMS metrics)

@elasticmachine
Copy link

elasticmachine commented Mar 29, 2022

💚 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 preview

Expand to view the summary

Build stats

  • Start Time: 2022-05-12T07:58:13.949+0000

  • Duration: 21 min 13 sec

Test stats 🧪

Test Results
Failed 0
Passed 5
Skipped 0
Total 5

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

@mtojek mtojek changed the title Initial skeleton along with 4 data streams structure Oracle WebLogic: Initial skeleton along with 4 data streams structure Mar 29, 2022
@@ -1,5 +1,5 @@
# Default owners
* @elastic/integrations-developer-experience
* @elastic/ecosystem
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please rebase it against the main branch? It looks like you pulled some other changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure. Its done

@ruflin
Copy link
Member

ruflin commented Mar 29, 2022

log (Admin server logs, Managed server logs, Domain logs, HTTP access logs)

This sounds like 4 different data streams?

Can we please only start with a single data stream overall and not add skeleton for others?

@nirav-elastic
Copy link

@ruflin , The reason for adding just the skeleton of all the data streams(i.e. 4) was to get a confirmation on the logical grouping and division of the data streams from reviewers. I understand we want to have multiple PRs for different data streams, do you have ideas on how do we ensure that at the same time we also get this logical data stream grouping/distribution reviewed at this early stage?

@nirav-elastic
Copy link

nirav-elastic commented Mar 29, 2022

@ruflin , The reason for adding just the skeleton of all the data streams(i.e. 4) was to get a confirmation on the logical grouping and division of the data streams from reviewers. I understand we want to have multiple PRs for different data streams, do you have ideas on how do we ensure that at the same time we also get this logical data stream grouping/distribution reviewed at this early stage?

One possible solution could be, this PR we just include 1 data stream skeleton but mention the logical grouping of the rest of the data streams as well in the description, and those rest of the data streams will be a separate PR. WYDT @ruflin ?

@mtojek
Copy link
Contributor

mtojek commented Mar 30, 2022

@nirav-elastic If you ask me, a reviewer, then I admit that you should apply this rule to EVERY integration PR :)

@ruflin
Copy link
Member

ruflin commented Mar 30, 2022

@nirav-elastic Ideally, the logical grouping we already have figured out before we start to work on the PRs in a Github issue. No code needs to be written for this. It is well possible that during implementation, we find that something needs to change but in general I expect us to be able to define it in advance.

The problem with having the skeleton in this PR means when we merge this PR, it already has more code inside then needed. Lets assume we decide to only implement 2 out of the 4 data streams, but now we already have a skeleton released for all 4. The idea is, that each PR we merge is a releasable unit. A package can be released with supporting just 1 data stream and later we add more.

The only place where I think data streams might overlap is in the dashboards which is not ideal but I'm sure we can solve these conflicts.

@sunny-elastic
Copy link
Contributor Author

@nirav-elastic Ideally, the logical grouping we already have figured out before we start to work on the PRs in a Github issue. No code needs to be written for this. It is well possible that during implementation, we find that something needs to change but in general I expect us to be able to define it in advance.

The problem with having the skeleton in this PR means when we merge this PR, it already has more code inside then needed. Lets assume we decide to only implement 2 out of the 4 data streams, but now we already have a skeleton released for all 4. The idea is, that each PR we merge is a releasable unit. A package can be released with supporting just 1 data stream and later we add more.

The only place where I think data streams might overlap is in the dashboards which is not ideal but I'm sure we can solve these conflicts.

@ruflin : Agreed! , lets do this way, we will update this PR with 1 data stream let say here for logs, which includes everything related to logs data stream as a whole, so could be easy to test entirely as a releasable unit, and will create subsequent PR's with the remaining individual data streams for this integration, and we can follow this as a standard practice over the other integrations as well.

@ruflin
Copy link
Member

ruflin commented Mar 30, 2022

@sunny-elastic The part I don't understand yet is why there is just 1 logs data stream. It seems we have 4 different data streams for logs: "Admin server logs, Managed server logs, Domain logs, HTTP access logs". Are these all in a single log file?

@sunny-elastic
Copy link
Contributor Author

@sunny-elastic The part I don't understand yet is why there is just 1 logs data stream. It seems we have 4 different data streams for logs: "Admin server logs, Managed server logs, Domain logs, HTTP access logs". Are these all in a single log file?

@ruflin as the log format for Admin Server log, Managed Server log, and Domains Logs is same, though these are all different files, it makes more sense to keep it under the one datastream. Also it has minimal set of fields to target and can be accommodated into 1 data stream as per relevance. WDYT !

@ruflin
Copy link
Member

ruflin commented Mar 31, 2022

I don't think the shared log format should be deciding factor here, more the consumption and retention. Will there be different visualisations for the different logs? May users want to collect only one of the log files? Is it possible that a user want to keep the adming server logs for 1 year and the domain logs for 1 month?

@sunny-elastic
Copy link
Contributor Author

I don't think the shared log format should be deciding factor here, more the consumption and retention.

Will there be different visualisations for the different logs?

Yes there will be 3 different visualizations (HTTP logs , Server Logs and Domain Logs)

May users want to collect only one of the log files?

User will be able to configure multiple log files (Add or Remove) when required.

Is it possible that a user want to keep the adming server logs for 1 year and the domain logs for 1 month?

We are using the filebeat log input which will be reading all the logs from the entire log file once the integration is configured, hence the logs for all the types will be kept in the same timeframe.

@ruflin
Copy link
Member

ruflin commented Apr 4, 2022

We are using the filebeat log input which will be reading all the logs from the entire log file once the integration is configured, hence the logs for all the types will be kept in the same timeframe.

That is the part I didn't get. There seem to be 3 different log files going into 3 data streams. If this it the case, the user can always choose how long to keep each.

@sunny-elastic
Copy link
Contributor Author

We are using the filebeat log input which will be reading all the logs from the entire log file once the integration is configured, hence the logs for all the types will be kept in the same timeframe.

That is the part I didn't get. There seem to be 3 different log files going into 3 data streams. If this it the case, the user can always choose how long to keep each.

Ok understood. So lets split logs into 4 different data-streams with the following name

  • http_access
  • domain
  • managed_server
  • admin_server

will try to cover each data stream implementation in an individual PR's.

@mtojek
Copy link
Contributor

mtojek commented Apr 5, 2022

@sunny-elastic Could you please look at the comments in the design doc? I identified a few gaps in data stream coverage, which will result in more/different data streams.

@elasticmachine
Copy link

elasticmachine commented May 10, 2022

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (0/0) 💚
Files 100.0% (0/0) 💚 3.597
Classes 100.0% (0/0) 💚 3.597
Methods 30.769% (4/13) 👎 -57.477
Lines 100.0% (0/0) 💚 10.887
Conditionals 100.0% (0/0) 💚

@yug-rajani yug-rajani marked this pull request as draft June 7, 2022 08:57
@kush-elastic
Copy link
Collaborator

Closing this PR as logs data-stream was split up into multiple PRs as discussed in the comment #2910 (comment). All the parts are now merged and the linked issue (#2823) has been closed.

Thanks a lot @agithomas, @mtojek, and @akshay-saraswat for taking out time to review the PRs and providing valuable feedback!

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.

None yet

6 participants