Skip to content

Outlook activity datastream for o365 package#11937

Merged
ritalwar merged 8 commits intoelastic:mainfrom
ritalwar:o365_outlook_activity
Dec 16, 2024
Merged

Outlook activity datastream for o365 package#11937
ritalwar merged 8 commits intoelastic:mainfrom
ritalwar:o365_outlook_activity

Conversation

@ritalwar
Copy link
Copy Markdown
Contributor

@ritalwar ritalwar commented Dec 2, 2024

  • Enhancement

Proposed commit message

Initial draft of the o365_metrics package with the outlook_activity data stream.

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.
  • I have verified that Kibana version constraints are current according to guidelines.
  • I have verified that any added dashboard complies with Kibana's Dashboard good practices

How to test this PR locally

Related issues

Screenshots

@ritalwar ritalwar self-assigned this Dec 2, 2024
@elastic-vault-github-plugin-prod
Copy link
Copy Markdown

🚀 Benchmarks report

To see the full report comment with /test benchmark fullreport

@andrewkroh andrewkroh added the New Integration Issue or pull request for creating a new integration package. label Dec 2, 2024
@lucian-ioan lucian-ioan self-requested a review December 2, 2024 20:54
Copy link
Copy Markdown
Contributor

@lucian-ioan lucian-ioan left a comment

Choose a reason for hiding this comment

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

Some minor nits and suggestions, otherwise looks good.

@ritalwar ritalwar marked this pull request as ready for review December 6, 2024 06:48
@ritalwar ritalwar requested a review from lucian-ioan December 6, 2024 06:50
Copy link
Copy Markdown
Contributor

@lucian-ioan lucian-ioan left a comment

Choose a reason for hiding this comment

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

LGTM. I think we can ship this as the first one.

- name: o365metrics.outlookactivity
type: group
fields:
- name: meeting_created
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

naming conventions recommend that units like count are appended to field names as a suffix e.g. meeting_created.count

https://www.elastic.co/guide/en/beats/devguide/7.17/event-conventions.html#units

@@ -0,0 +1,35 @@
- name: o365metrics.outlookactivity
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please can we add units, dimensions, metric types to these fields?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We plan to address TSDB-related changes later, once a few datastreams are merged. We've also asked for help to perform some service-related activities to generate richer API responses, which will help us implement these changes effectively.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is units done and only dimension and metric types pending?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have suffixed the names with .count where applicable. However, the unit value count is not allowed due to validation constraints, which specify that the unit must be one of the following: "byte", "percent", "d", "h", "m", "s", "ms", "micros", or "nanos".

name: o365_metrics
title: Microsoft Office 365 Metrics
version: "0.1.0"
description: Collect metrics from Microsoft Office 365 with Elastic Agent.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should we add (under-development) at the end?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated, and it looks like:
Screenshot 2024-12-16 at 10 56 43 AM

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i don't understand why we need this when we have the tech preview badge

@lalit-satapathy
Copy link
Copy Markdown
Contributor

  • For the field naming we have now as o365metrics.*. One alternative is o365.metrics. Any suggestion, which is preferable? Also the O365 audit package has fields named as: o365.audit.*, if we want to be consistent to that.
  • For data stream naming, also lets define a consistent scheme between these choices and something which works for all future data_stream names:
    • outlookactivity vs.
    • outlook.activity vs.
    • outlook_activity
      (Also please note it may be confusing to create a future data_stream name for m365.active_users since, o365 is already being used in the beginning).

CC: @lucian-ioan

@ritalwar
Copy link
Copy Markdown
Contributor Author

  • For the field naming we have now as o365metrics.*. One alternative is o365.metrics. Any suggestion, which is preferable? Also the O365 audit package has fields named as: o365.audit.*, if we want to be consistent to that.

Yes, I think it's better to maintain consistency, so I've updated the naming to o365.metrics.

  • For data stream naming, also lets define a consistent scheme between these choices and something which works for all future data_stream names:

    • outlookactivity vs.
    • outlook.activity vs.
    • outlook_activity

For field naming, I have updated it to outlook.activity, following the structure o365.metrics.outlook.activity.*. However, for the datastream name, the naming convention validation does not allow the use of outlook.activity. To maintain clarity, I suggest sticking with outlook_activity for the datastream name.

@elastic-sonarqube
Copy link
Copy Markdown

Quality Gate failed Quality Gate failed

Failed conditions
17.4% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube

@elasticmachine
Copy link
Copy Markdown

💚 Build Succeeded

History

cc @ritalwar

Copy link
Copy Markdown
Contributor

@lalit-satapathy lalit-satapathy left a comment

Choose a reason for hiding this comment

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

Early development version. Approving to enable successive PR merge.

@ritalwar ritalwar merged commit 9838908 into elastic:main Dec 16, 2024
@elastic-vault-github-plugin-prod
Copy link
Copy Markdown

Package o365_metrics - 0.1.0 containing this change is available at https://epr.elastic.co/package/o365_metrics/0.1.0/

@andrewkroh andrewkroh added Integration:o365_metrics Microsoft Office 365 Metrics Team:Obs-InfraObs Observability Infrastructure Monitoring team [elastic/obs-infraobs-integrations] labels Dec 16, 2024
harnish-crest-data pushed a commit to chavdaharnish/integrations that referenced this pull request Feb 4, 2025
* Initial draft of the o365_metrics package with the `outlook_activity` data stream.
harnish-crest-data pushed a commit to chavdaharnish/integrations that referenced this pull request Feb 5, 2025
* Initial draft of the o365_metrics package with the `outlook_activity` data stream.
@ritalwar ritalwar deleted the o365_outlook_activity branch February 12, 2025 05:49
@andrewkroh andrewkroh removed the New Integration Issue or pull request for creating a new integration package. label Mar 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Integration:o365_metrics Microsoft Office 365 Metrics Team:Obs-InfraObs Observability Infrastructure Monitoring team [elastic/obs-infraobs-integrations]

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants