Remove variant & telemetry support for .NET Feature Management schema #421
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Is there a reason to still pass in values for status, allocation, variants, telemetry to the FeatureDefinition constructor at the end of the ParseDotnet method? It looks like they should be removed, as well as the temporary variables like |
src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs
Show resolved
Hide resolved
src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs
Show resolved
Hide resolved
My bad. I forget to remove them. |
jimmyca15
reviewed
Apr 10, 2024
src/Microsoft.FeatureManagement/ConfigurationFeatureDefinitionProvider.cs
Show resolved
Hide resolved
|
@jimmyca15 Should we update appsettings.json for all example projects to use Microsoft schema? Currently, I only updated the EvaluationDataToApplicationInsights example. |
yes |
jimmyca15
approved these changes
Apr 19, 2024
rossgrambo
added a commit
that referenced
this pull request
Sep 12, 2024
* Add variants for feature flags (#250) * in progress, add new classes for variants and define methods in featuremanager * Revert "Revert "Add cancellation token parameter to async feature management interfaces. (#131)" (#139)" This reverts commit e531863. * Revert "Revert "Added default value for cancellation token in interfaces to keep existing usage possible. (#133)" (#138)" This reverts commit 8f9a7e4. * fix any conflicts left from adding cancellationToken back * add in progress changes to allocation and featuredefinitionprovider * add examples for testing * fix adding new featuredefinition properties from featuremanagement definition * progress adding getvariant logic classes * continued * remove repeated code in contextual targeting * fix version of contextual filter * more progress on getting the contextual allocator to work * about to test getvariant * add example to test * add snapshot changes * variant can be detected and retrieved from getvariantasync * progress on allocation logic, add comments where consideration needed * add use of optionsresolver for reference, todo work on isenabledasync between customer use and variant use * All working except couple TODOs, need to add unit tests * remove some comments, add null check where needed * update todo comments * fix line eols * add unit test, in progress * TODOs in progress, need to restructure featurevariantassigner design * fix seed logic * update comments, status logic * remove unnecessary files for custom assigners, fix featuremanager methods and interfaces to match * fix naming from allocator to assigner for classes and files * cleanup extra methods, todo config section logic * in progress adding configurationsection returned when using configurationvalue * continuation of last commit * working return for configvalue * move logic to featuremanager for assigning * remove unused assigner classes * add new configurationsection to handle return for variant * null error, in progress new configurationsection class * fix old bug * progress on unit tests * more null check changes, test fixes * reset examples changes * Revert "Revert "Revert "Added default value for cancellation token in interfaces to keep existing usage possible. (#133)" (#138)"" This reverts commit d087e7b. * Revert "Revert "Revert "Add cancellation token parameter to async feature management interfaces. (#131)" (#139)"" This reverts commit c1451d3. * add comments for new classes * fix comments for public classes again * update comments, default values * fix variantconfigurationsection, comments in definitionprovider * fix using statements, null checks * fix unit test failures with servicecollectionextensions * add revisions: fix namepaces, add exceptions tests, combine percentage logic, fix comments, add cancellationtoken to new interface * change context accessor logic * fix comments for default variants * PR revisions * change class names, PR fixes * fix edge case percentage targeting * rename allocation classes, remove exceptions and add warning logs, prioritize inline value for variant config, more revisions * refactor isenabled to remove boolean param * change configurationvalue to IConfigurationSection instead of string * fix enabledwithvariants logic * PR revisions, fix logic in new methods from last commit * set session managers last in flow * make false explicit for status disabled or missing definition * fix constructor default params, move session managers logic, pr revisions * fix comment * fix resolvedefaultvariant, isexternalinit error * add back 3.1 * Apply suggestions from code review Co-authored-by: Jimmy Campbell <jimmyca@microsoft.com> * isexternalinit comments, remove resolvedefault helper * remove binding, fix featuredefinitionprovider issues * change to Debug.Assert from Assert * update method name * remove parseenum, add ConfigurationFields class * test failing, fixed PR revisions * fix invalid scenarios test * simplify context in test * remove unused using * remove unused param * Clarify how From and To bounds work in PercentileAllocation Co-authored-by: Ross Grambo <rossgrambo@microsoft.com> * fix error messages * add feature name as default seed with allocation prefix * Update src/Microsoft.FeatureManagement/FeatureManager.cs Co-authored-by: Jimmy Campbell <jimmyca@microsoft.com> --------- Co-authored-by: Jimmy Campbell <jimmyca@microsoft.com> Co-authored-by: Ross Grambo <rossgrambo@microsoft.com> * Adds GetFeatureNamesAsync to IVariantFeatureManager and uses the interface in FeatureManagerSnapshot (#270) * Adds telemetry pipeline (#259) * Adds telemetry pipeline. Adds evaluationevent, publisher interface, and a publisher instance for app insights. * Use await insead of .result * Reverting unused dependency removal * Update src/Microsoft.FeatureManagement/FeatureDefinition.cs Co-authored-by: Jimmy Campbell <jimmyca@microsoft.com> * Update src/Microsoft.FeatureManagement/FeatureDefinition.cs Co-authored-by: Jimmy Campbell <jimmyca@microsoft.com> * Update src/Microsoft.FeatureManagement.AppInsightsTelemetryPublisher/TelemetryPublisherAppInsights.cs Co-authored-by: Jimmy Campbell <jimmyca@microsoft.com> * Removed factory pattern for publisher DI * Update src/Microsoft.FeatureManagement/FeatureManager.cs Co-authored-by: Jimmy Campbell <jimmyca@microsoft.com> * Resolve misc. comments * Adjusts descriptions for FeatureDefinition properties * Rename AppInsights to ApplicationInsights * Resolves project reference in sln * Adjusts ServiceCollectionExtensions to use TryAddSingleton * Removes langversion from .csproj * Temp * Adds explicit null checkts before publishing fields to app insights * Addressing misc. comments * Removing project reference for example project * Syncing telemetry and variants * Adds null check for feature definitions * Adjusted TelemetryPublishers to no longer be inserted into DI * Resolving comments * Removing Application Insights project * Converts some variables to inline * Resolving misc. comments * Moves AddTelemetryPublisher to be an extension method * Removing IVariantFeatureManager GetFeatureNamesAsync definition, as it should it it's own PR * Remove unused dependency * Resolving comments * Remove invisible character changes * Remove Dependency Injection package * Update src/Microsoft.FeatureManagement/FeatureManagerSnapshot.cs * Update src/Microsoft.FeatureManagement/IVariantFeatureManager.cs * Persists cancellation token to PublishEvent and adds null check for null publisher collection * Adds TargetingContext to the EvaluationResult * Resolving comments * Removes TargetingContext from EvaluationEvent for now * Moves tags, etag, and label under 'TelemetryMetadata' * Adjusts telemetry metadata to already be a flattened dictionary * Removes bind in favor of ToDictionary. Removes unused using --------- Co-authored-by: Jimmy Campbell <jimmyca@microsoft.com> * Application Insights Publisher (#281) * Adds Application Insights publisher * Change readonly to const * Update src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryPublisher.cs Co-authored-by: Jimmy Campbell <jimmyca@microsoft.com> * Update src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryPublisher.cs Co-authored-by: Jimmy Campbell <jimmyca@microsoft.com> --------- Co-authored-by: Jimmy Campbell <jimmyca@microsoft.com> * Adds example library for Application Insights * Adds readme to example * Updates readme and persists telemetry metadata by placing it in the correct field * Update README file for variants (#264) * update readme with variants, first draft * small summary revisions * fix wording of summary * fix code description * Apply suggestions from code review Co-authored-by: Jimmy Campbell <jimmyca@microsoft.com> * revisions * PR revisions, describe Allocation properties, fix descriptions and titles * clarify configurationvalue possible values * clarify seed description * remove all mentions of On filter * fix example for override, clarify allowing no configurationvalue or reference * change statusoverride example * Apply suggestions from code review Co-authored-by: Jimmy Campbell <jimmyca@microsoft.com> * revisions to summaries, fix allocation table --------- Co-authored-by: Jimmy Campbell <jimmyca@microsoft.com> * Edits to README for variants (#289) * add edits for variants * fix wording * Simplified javascript and other slight modifications * Adjusts IVarantManager to use ValueTask (#293) * remove conflict mark * add missing comment & [Fact] attribute * Updates. * Updates. * Improvements. * Add missing project in sln file. * Fix various solution issues * missing project inclusion in solution file * Project declared nullable support but wasn't written with it in mind * pack.ps1 script didn't reference newest project. * Remove nullable usage. Other updates. * remove & add empty line * resolve conflicts * Fix project added twice. * use const string and feature flag referece * Add Telemetry section * Made section casing consistent. * Add section for enabling telemetry on flags. * Update TOC. * Use relative links. * should -> must * Updates based on feedback. Reordered telemetry sections. * Add note. * Update README.md * Update telemetry schema in feature flags. * Merges variants and isenabled paths. Adds variant reason field. (#290) * Adds Reason field and adjusts evaluation logic * use enum AssignmentReason * add more testcases & remove some interal methods * fix typo * update comments * resolve comments * update comment for DisabledDefault * fix typo * update * update telemetry publisher * add reason when no allocation section * update * remove nullable * resolve comments * update comments --------- Co-authored-by: zhiyuanliang <zhiyuanliang@microsoft.com> * Undo whitespace changes from merge. * Readme clarifications. Added argument exception if feature definition telemetry is null when passed into app insights telemetry publisher. * Update property summary. * Prepare packages for 4.0.0-preview release. * Add preview label. Update telemetry package description. * Bumps Application Insights version (#342) * Updates example application insights version (#344) * Mention TargetingContext and fix wording in Variants section of readme (#330) * mention itargetingcontextaccessor for variant allocation * update changes * add reference to ITargetingContextAccessor to readme for allocation * update with new discussed changes, rename sections and introduce context sooner * fix wording of default * fix wording to avoid implying variants are assigned per feature * use the term assign when referring to variants returned for a user * use assign term in more places * small changes * Update README.md Co-authored-by: Jimmy Campbell <jimmyca@microsoft.com> * update section titles * resolve comments * fix typo * update link to allocating variants section --------- Co-authored-by: Jimmy Campbell <jimmyca@microsoft.com> * Adds TargetingContext to EvaluationEvent & Emits TargetingId in the App Insights Publisher (#347) * Adds TargetingId to EvaluationEvent * Adjusts EvaluationEvent to use TargetingContext instead of ITargetingContext * move public method to the front of private method (#349) * Adds extension methods for TrackEvent and TrackMetric (#348) * Adds extension methods for TrackEvent and TrackMetric * Adds copyright, crefs, and uses []= instead of .Add for properties * Update src/Microsoft.FeatureManagement.Telemetry.ApplicationInsights/ApplicationInsightsTelemetryExtensions.cs Co-authored-by: Jimmy Campbell <jimmyca@microsoft.com> * Added null check, moved shared logic to helper functions * Removed TrackMetric extension for now --------- Co-authored-by: Jimmy Campbell <jimmyca@microsoft.com> * Adds missing cref (#352) * Adjusts private methods from Task to ValueTask (#353) * Adds targeting middleware and targeting initializer (#350) * Adds targeting middleware and targeting initializer * Added logger and added prefix to targetingId * Apply suggestions from code review Co-authored-by: Jimmy Campbell <jimmyca@microsoft.com> * Adjustments from comments --------- Co-authored-by: Jimmy Campbell <jimmyca@microsoft.com> * unify the parse methods * update * Adjusts example to use middleware and initializer for targeting id (#357) * Adjusts example to use middleware and initializer for targeting id * Update examples/EvaluationDataToApplicationInsights/README.md * remove unused package * Feature-based Injection (#335) * init * draft * use ValueTask * support factory method * add example * update * Update * Update * update example * match variant name or configuration value * update to the latest design * merge with preview branch * resolve comments * rename to VariantService * update & add comments * remove POC example * add testcases & use method name GetServiceAsync * update comments * add variant service cache * resolve comments * throw exception for duplicated registration * add testcase * remove unused package * update comment * set feature name in constructor * adjust-tab-size (#366) * Merge main to preview (#373) * Add a constructor without FeatureManagementOption parameter for FeatureManager (#363) * add new constructor * update * Target on .NET 8.0 (#365) * target on .NET 8.0 * remove file * add net8.0 for Microsoft.FeatureManagement.AspNetCore * update package version * update * Add feature management schema for main branch (#362) * schema file added * README update * update * update * update README & remove the Microsoft schema file * update readme * update * Updates test schema and adjusts title (#372) * Support Microsoft Feature Management schema for main branch (#370) * use snake case * do not support root fall back for MS schema & remove EnsureInit * update * rename variable * update .NET FM schema * re-add schema link * add whitespace (#374) * update --------- Co-authored-by: Ross Grambo <rossgrambo@microsoft.com> * add missing param comment (#380) * Version bump * Adds .Telemetry.ApplicationInsights.AspNetCore to build packages * Support Microsoft Feature Management schema for preview branch (#375) * support microsoft feature flag schema v2 * add more testcases * add empty lines * add schema & update README * update schema * update (#398) * Optional Cancellation Token (#395) * make cancelltation optional for feature manager & add cancellation token for session manager * revert breaking change in ISessionManager * remove unused package * Update README to mention the usage of feature-based injection (variant service) (#388) * update README * update * update * update * update * fix typo * update * update * Avoid redundant validation for cached TargetingFilterSettings (#387) * do validation during binding parameters * adjust method order * Update target framework for telemetry packages (#413) * update target framework * update language version * Adjusts namespace to simply ~.Telemetry * Adjusts ApplicationInsights extension methods namespace * Adjusts class name of TelemetryClientExceptions * Removed changes to FM.Telemetry.ApplicationInsights.AspNetCore namespace * Ensure the consistency of targeting evaluation across CPU architectures. (#405) * check system endianness * remove period * update reverse method * update comment * adjust comment * version bump (#420) * Move warning deeper into variant code * Adjusted warning message * Boolean zen * Updating from comments * Adjusted wording * Formatting update * Promote Microsoft FM schema in README (#414) * replace examples using Microsoft schema * update * correct section name * update * update schema reference link * Remove variant & telemetry support for .NET Feature Management schema (#421) * reorg test suite * remove variant and telemetry & reorg test suite * update * update constructor * update appsettings.json of EvaluationDataToApplicationInsights example project * update appsetting.json for all examples * add dependency for telemetry aspnetcore package (#439) * Support variant for FeatureTagHelper (#407) * add variant for feature tag helper * update * Negate also applies for variant * correct comment * update comment * update README * update README * throw exception for Requirement All * update readme * update * Moves telemetry publishing to Activity * Update src/Microsoft.FeatureManagement/FeatureManager.cs Co-authored-by: Juliano Leal Goncalves <julealgon@gmail.com> * Update src/Microsoft.FeatureManagement/FeatureManager.cs Co-authored-by: Juliano Leal Goncalves <julealgon@gmail.com> * Resolving comments * avoid null reference (#457) * Some formatting updates * Adjusted exception & exception message * Update src/Microsoft.FeatureManagement/FeatureManager.cs Co-authored-by: Juliano Leal Goncalves <julealgon@gmail.com> * Adjusts hosted service to use the ServiceProvider instead of manually creating the publisher * Update src/Microsoft.FeatureManagement/FeatureManager.cs Co-authored-by: Zhiyuan Liang <141655842+zhiyuanliang-ms@users.noreply.github.com> * Resolving comments * Moved conditional checks * Switched to standard null checks * Adjusted to use activity parent instead of thread check and adjusted null or empty event field check * Updated string null check * Updates activity event to be PascalCase * update example name * Update Variant Example Application (#437) * add example for variant based injection * readme added * not use third-party api * update * update README * fix typo * use latest api * Switches to Activity for storing TargetingId and removes Microsoft.FeatureManagement.Telemetry.AspNetCore project * Removes Microsoft.FeatureManagement.Telemetry.ApplicationInsights.AspNetCore * Adds null check in middleware * Added debug logs for unexpected nulls * Respect .NET and Microsoft feature management schemas in configuration (#470) * respect all feature management schemas * add testcase * add testcase * correct test case * simplify methods * remove unused package * unify newline usage * remove useless code & rename variables * reuse ParseEnum * rename variable * Switching log from debug to warning (#471) * Switching log from debug to warning * Adjusted warning logs * adjust spacing * version bump 4.0.0-preview4 (#476) * fix typo in comment (#480) * Adjusts builder extension methods to be friendlier (#487) * Adds and adjusts builder extension methods * Removed UseFeatureManagement and adjusted name of AddAppInsightsTelemetryPublisher * Adjusts examples * Use ITargetingContext when calling GetVariantAsync (#484) * add variant feature manager extensions * use TContext to avoid future breaking change * TargetingContext in EvaluationEvent * update comments & not use ambient context in contextual case * update comment * use ITargetingContext & check runtime context type * use TargetingCotnext for private method * use var instead of type * revert change on ContextualTestFilter * update comment * update comment * remove configuration reference from variant feature flag (#488) * Renames and updates EvaluationDataToAppInsights example (#490) * Renames project and cleans up startup code + auth * Update examples/VariantAndTelemetryDemo/Program.cs Co-authored-by: Zhiyuan Liang <141655842+zhiyuanliang-ms@users.noreply.github.com> --------- Co-authored-by: Zhiyuan Liang <141655842+zhiyuanliang-ms@users.noreply.github.com> * For examples and tests- fixes warnings, updates packages, and updates… (#491) * Renames project and cleans up startup code + auth * For examples and tests- fixes warnings, updates packages, and updates .net versions * Switched to var * Reverted construtor away from primary constructor style * Missing semicolon * use TimeProvider (#452) (#492) Co-authored-by: Zhiyuan Liang <141655842+zhiyuanliang-ms@users.noreply.github.com> * Removes preview (#493) * Removes unused using statements and cleans up spacing. (#496) --------- Co-authored-by: Amer Jusupovic <32405726+amerjusupovic@users.noreply.github.com> Co-authored-by: Jimmy Campbell <jimmyca@microsoft.com> Co-authored-by: zhiyuanliang <zhiyuanliang@microsoft.com> Co-authored-by: Zhiyuan Liang <141655842+zhiyuanliang-ms@users.noreply.github.com> Co-authored-by: Juliano Leal Goncalves <julealgon@gmail.com> Co-authored-by: AMER JUSUPOVIC <ajusupovic@microsoft.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why this PR?
In previous versions, the primary schema for the feature management library was the .NET feature management schema. Starting from v4.0.0, new features including variants and telemetry will not be supported for the .NET feature management schema. #408
This PR follows up #414
Visible Changes
Variant and telemetry are no longer supported for the .NET Feature Management schema.
The removed fields of feature flag declaration include
Status,Variants,Allocation,TelemetryThe test suite is also re-organized.