Skip to content

Add metadata configuration object to TelemetryItem, for use in .track(...) function to enable customizing event property truncation threshold #2419

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

Conversation

MgenGlder
Copy link

@MgenGlder MgenGlder commented Sep 20, 2024

Note

This is an early POC and draft. I'm looking for feedback and working through the best way to approach this addition.

References #2383

Todo

  • Implement sanitization options for all event types, not just Event.
    • Dependency
    • Exception
    • Metric
    • PageView
    • Trace
    • PageViewPerformance
  • Add integration test around calling the .track function with metadata passed in, and having the maxLength configuration be respected.
  • Update public facing README.md docs with addition of configuration object and instructions and recommendations for how to use it.

Nice-To-Haves / Next Iteration Goals

  • It would be nice to have this configuration set when instantiating the ApplicationInsights client, and not passed every time you call the .track function. I'm limited by my understanding of the codebase at present, but I will look to attempt to add this in future iterations / PRs.

Summary

Enables customization for properties bag value truncation through a new metadata configuration object, passed through the .track(...) function.

Additions:

  • Add maxLength configuration to sanitizer.
  • Add metadata dictionary object to house user configurations when calling .track(...) function.

Context

We're using the ApplicationsInsights-web-basic npm package to make requests for an internal service that mimics the behavior of the ApplicationInsights managed service, sans the property size limit. We'd like to make it so you can configure the max length before truncation.

Use

You should now be able to pass a metadata object within your TelemetryItem when calling .track(...) via the ApplicationInsights client 👇🏾

let properties = [/* list of properties */]
let measurements = [/* list of measurements */]

let client = new ApplicationInsights({
            /* ... Other configuration properties ... */
            instrumentationKey: someKey,
            endpointUrl: someCustomUrl
            },
 });

this.client.track({
            name,
            tags: this.tags,
            data: {...properties, ...measurements},
            baseType: 'EventData',
            baseData: {name, properties, measurements},
            metadata: {
               "maxLength": 12000
           }
 });

Notes

This can be leveraged in the future for passing down custom configurations that can be used to override hard coded constants.

- Also add `maxLength` configuration to sanitizer.
@MgenGlder MgenGlder changed the title Add metadata configuration object to TelemetryItem Add metadata configuration object to TelemetryItem, for use in .track(...) function Sep 20, 2024
@MgenGlder MgenGlder changed the title Add metadata configuration object to TelemetryItem, for use in .track(...) function Add metadata configuration object to TelemetryItem, for use in .track(...) function to enable customizing event property truncation threshold Sep 20, 2024
@MSNev
Copy link
Collaborator

MSNev commented Sep 20, 2024

We can't just "add" properties to the ITelemetryItem as this library is also used by an internal extension and having something else (while optional) would cause a clash.

There are also longer term plans that are starting to solidify on how we might go about providing OpenTelementry support / API compatibility, and this approach would clash with some of the ideas that are circulating.

Tracing this back to where it gets called it looks like it occurring in the Sender via the constructEnvelope function which is currently looking up a constant set of EnvelopeTypeCreator. Sooo...

Perhaps, introducing a configuration on the "ISenderConfig" (which will then be unique to Application Insights, so no clash concerns) that would allow you to provide either an override implementation of the entire EnvelopeCreator set or a specific "type".

Other options (which we have considered in the past and we have for the 1ds-post implementation), is to introduce a serializer interface (which is not a small change) so that it can be (and we do in some internal code extensions) replaced with something else -- it primary use case was to provide "sanitization" in the form of Hashing out configured fields.... We may still introduce this as well, so any changes in this code is a little messy.

Other options you might want to consider.

  • Create your own "Sender" implementation (this code really is the stuff that packages the event and handles server communication)
  • Once you have done the above, you can then create your own "SKU" (the main applicationinsights-web or AISKU) as it's really just a collection of the default extensions, but that also might get tricky if we add some fundamental change (like the Promise connection strings that we just added and the dynamic config stuff)
  • Another random thought (and I've not dug through the code to make sure we have not blocked this capability), you can add / remove "plugins" (which includes the "Sender") after initialization...
  • And the way we "initialize" (and execute) the "plugins" is based on the "priority" value so if you created / added your "plugin" and gave it a lower priority than the 1001 (the default "Sender" priority), yours would be called first and then if you never called processNext our "Sender" code would never run -- this later one might be the most longer term compatible option...

Copy link
Collaborator

@MSNev MSNev left a comment

Choose a reason for hiding this comment

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

Blocking for now based on my previous comments

@MgenGlder
Copy link
Author

MgenGlder commented Sep 23, 2024

@MSNev Thanks for looking at this.

I have a few questions that will help me navigate the rework:

  • Do you have any concrete examples of creating an ISenderConfig, and only implementing it for ApplicationInsights web, such that it doesn't impact other internal applications?
  • Also, is it possible to name the custom field something that would not clash with internal plans / tools?
  • We currently use metadata internally and have found it useful to hold custom data, is this a concept you already are utilizing?
  • Do you have any documentation or diagrams of the structure of this repo and its processes?

@MSNev
Copy link
Collaborator

MSNev commented Sep 25, 2024

Sorry, I was unexpectedly OOF yesterday.

Do you have any concrete examples of creating an ISenderConfig, and only implementing it for ApplicationInsights web, such that it doesn't impact other internal applications?

Simplistically, any extension (thing that extends the BaseTelemetryPlugin) has it's own configuration that during initialization is handled by specifying it's configuration in the extensionConfig.XXXX (where XXXX is the identifier assigned to the extension), so that all of it's configuration is isolated from everything else.

One (gotya) that we have to deal with (that you won't necessarily for your extension), is that historically (and still supported), is that if the configuration is not specified in any extensionConfig.XXXX then the code falls back looking for the "default" values in the root of the configuration object (so the root becomes a mis-mash of all configurations), so when specifying configuration options for "new" extensions we make particular attention of the "names" to try to not introduce and configs that have different meanings between different components. Apart from some explicit instances, we generally do NOT define all of them directly in the IConfiguration or IConfig root interface definitions (so from a TypeScript perspective they are not supported, but from a JavaScript runtime they are)

Also, is it possible to name the custom field something that would not clash with internal plans / tools?

As above as long as you "always" provide an extensionConfig and pout everything in there you should be fine.

We currently use metadata internally and have found it useful to hold custom data, is this a concept you already are utilizing?

The 1ds extensions (1ds-core-js and 1ds-post -- part of the internal MS only usages) we do have a metadata but it has a different meaning and is actually sent as ext.metadata, so it doesn't explicitly use the same thing you are, but we avoid adding anything to the ITelemetryItem (at least in the core) -- there is an internal component / extension which explicitly adds / extends the ITelemetryItem but that extension also detects the presence of this "extension" and either deletes it or drops the "event" before continuing on the downstream processing processNext().

Do you have any documentation or diagrams of the structure of this repo and its processes?

Not beyond what is in the repo itself, we are working on building it out (in the docs folder which is deployed to https://microsoft.github.io/ApplicationInsights-JS/) and in the readme for creating your own extension

Copy link

This PR will be closed in 30 days. Please remove the "Stale" label or comment to avoid closure with no action.

@github-actions github-actions bot added stale and removed stale labels May 24, 2025
@MSNev MSNev added the stale label May 27, 2025
@github-actions github-actions bot removed the stale label May 28, 2025
@MSNev
Copy link
Collaborator

MSNev commented May 28, 2025

closing as the auto stale keeps unstaling

@MSNev MSNev closed this May 28, 2025
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.

2 participants