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

feature: Azure Application Insights Integration #2691

Conversation

xoscar
Copy link
Collaborator

@xoscar xoscar commented Jun 8, 2023

This PR adds the Azure Application Insights data store integration

Changes

  • Allows data stores to have a direct vs a collector config
  • Enables Azure App insights as data store
  • Enables config and start up for new data store

Fixes

Checklist

  • tested locally
  • added new dependencies
  • updated the docs
  • added a test

https://www.loom.com/share/6ff8f264bb7748329050ca0132c71aa7

@xoscar xoscar linked an issue Jun 8, 2023 that may be closed by this pull request
@xoscar xoscar changed the title 2590 add microsoft application insights as a supported trace datastore feature: Azure Application Insights Datastore Integration Jun 8, 2023
@xoscar xoscar self-assigned this Jun 8, 2023
@xoscar xoscar added the tracing backend Improve or implement support for a new tracing backend label Jun 8, 2023
@xoscar xoscar marked this pull request as ready for review June 8, 2023 22:21
@xoscar xoscar changed the title feature: Azure Application Insights Datastore Integration feature: Azure Application Insights Integration Jun 8, 2023
type: boolean
accessToken:
type: string
connectionType:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think this connection type is something we can move to be part of the base data store object, I don't want to do it now but it will be very useful in the future

@@ -202,21 +223,26 @@ func (ds DataStore) GetID() id.ID {
}

func (ds DataStore) IsOTLPBasedProvider() bool {
if ds.Type == DatastoreTypeAzureAppInsights {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In the future, I'd prefer having each integrator decide if they are otlp or not based on the values and such. this patch is temporary

Copy link
Member

@mathnogueira mathnogueira 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!

@xoscar xoscar merged commit e16ed87 into main Jun 9, 2023
24 checks passed
@xoscar xoscar deleted the 2590-add-microsoft-application-insights-as-a-supported-trace-datastore branch June 9, 2023 13:39
Copy link
Collaborator

@kdhamric kdhamric left a comment

Choose a reason for hiding this comment

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

Looks great!!

mathnogueira pushed a commit that referenced this pull request Jun 13, 2023
* save commit

* feature: Azure Application Insights Datastore Integration
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tracing backend Improve or implement support for a new tracing backend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Microsoft Application Insights as a supported trace datastore
3 participants