Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: erroneous warning about prop overwrite #924

Merged
merged 2 commits into from
Sep 19, 2023
Merged

fix: erroneous warning about prop overwrite #924

merged 2 commits into from
Sep 19, 2023

Conversation

craigpastro
Copy link
Member

This PR

When adding flagdProperties to the context in preparation for evaluation return a new context so that it doesn't affect any references to the current context.

Without this change:

$ make run
make run-flagd
cd flagd; go run main.go start -f file:../config/samples/example_flags.flagd.json

                 ______   __       ________   _______    ______      
                /_____/\ /_/\     /_______/\ /______/\  /_____/\     
                \::::_\/_\:\ \    \::: _  \ \\::::__\/__\:::_ \ \    
                 \:\/___/\\:\ \    \::(_)  \ \\:\ /____/\\:\ \ \ \   
                  \:::._\/ \:\ \____\:: __  \ \\:\\_  _\/ \:\ \ \ \  
                   \:\ \    \:\/___/\\:.\ \  \ \\:\_\ \ \  \:\/.:| | 
                    \_\/     \_____\/ \__\/\__\/ \_____\/   \____/_/ 

2023-09-18T19:16:39.581-0700    info    cmd/start.go:117        flagd version: dev (HEAD), built at: unknown    {"component": "start"}
2023-09-18T19:16:39.581-0700    info    file/filepath_sync.go:37        Starting filepath sync notifier {"component": "sync", "sync": "filepath"}
2023-09-18T19:16:39.582-0700    info    flag-evaluation/connect_service.go:203  metrics and probes listening at 8014    {"component": "service"}
2023-09-18T19:16:39.582-0700    info    file/filepath_sync.go:66        watching filepath: ../config/samples/example_flags.flagd.json       {"component": "sync", "sync": "filepath"}
2023-09-18T19:16:39.582-0700    info    flag-evaluation/connect_service.go:183  Flag Evaluation listening at [::]:8013  {"component": "service"}
2023-09-18T19:16:41.258-0700    warn    eval/json_evaluator.go:368      overwriting $flagd properties in the context    {"component": "evaluator", "evaluator": "json"}
2023-09-18T19:16:41.259-0700    warn    eval/json_evaluator.go:368      overwriting $flagd properties in the context    {"component": "evaluator", "evaluator": "json"}
2023-09-18T19:16:41.259-0700    warn    eval/legacy_fractional_evaluation.go:35 fractionalEvaluation is deprecated, please use fractional, see: https://flagd.dev/concepts/#migrating-from-legacy-fractionalevaluation
2023-09-18T19:16:41.259-0700    warn    eval/json_evaluator.go:368      overwriting $flagd properties in the context    {"component": "evaluator", "evaluator": "json"}

With this change:

$ make run
make run-flagd
cd flagd; go run main.go start -f file:../config/samples/example_flags.flagd.json

                 ______   __       ________   _______    ______      
                /_____/\ /_/\     /_______/\ /______/\  /_____/\     
                \::::_\/_\:\ \    \::: _  \ \\::::__\/__\:::_ \ \    
                 \:\/___/\\:\ \    \::(_)  \ \\:\ /____/\\:\ \ \ \   
                  \:::._\/ \:\ \____\:: __  \ \\:\\_  _\/ \:\ \ \ \  
                   \:\ \    \:\/___/\\:.\ \  \ \\:\_\ \ \  \:\/.:| | 
                    \_\/     \_____\/ \__\/\__\/ \_____\/   \____/_/ 

2023-09-18T19:20:29.011-0700    info    cmd/start.go:117        flagd version: dev (HEAD), built at: unknown    {"component": "start"}
2023-09-18T19:20:29.012-0700    info    file/filepath_sync.go:37        Starting filepath sync notifier {"component": "sync", "sync": "filepath"}
2023-09-18T19:20:29.013-0700    info    flag-evaluation/connect_service.go:203  metrics and probes listening at 8014    {"component": "service"}
2023-09-18T19:20:29.013-0700    info    flag-evaluation/connect_service.go:183  Flag Evaluation listening at [::]:8013  {"component": "service"}
2023-09-18T19:20:29.014-0700    info    file/filepath_sync.go:66        watching filepath: ../config/samples/example_flags.flagd.json       {"component": "sync", "sync": "filepath"}
2023-09-18T19:20:32.784-0700    warn    eval/legacy_fractional_evaluation.go:35 fractionalEvaluation is deprecated, please use fractional, see: https://flagd.dev/concepts/#migrating-from-legacy-fractionalevaluation

Both logs were triggered by the request in the bug report:

$ curl -X POST "http://localhost:8013/schema.v1.Service/ResolveAll" \
  -d '{"context":{}}' -H "Content-Type: application/json"

Related Issues

Fixes #923.

Notes

Follow-up Tasks

How to test

Signed-off-by: Craig Pastro <pastro.craig@gmail.com>
@craigpastro craigpastro requested a review from a team as a code owner September 19, 2023 02:31
@netlify
Copy link

netlify bot commented Sep 19, 2023

Deploy Preview for polite-licorice-3db33c canceled.

Name Link
🔨 Latest commit 79fa5dc
🔍 Latest deploy log https://app.netlify.com/sites/polite-licorice-3db33c/deploys/6509c9d988d4b50008c8eb77

@craigpastro craigpastro changed the title bug: create new context when adding flagd properties fix: create new context when adding flagd properties Sep 19, 2023
@codecov
Copy link

codecov bot commented Sep 19, 2023

Codecov Report

Merging #924 (79fa5dc) into main (e1e7ca0) will decrease coverage by 0.09%.
Report is 1 commits behind head on main.
The diff coverage is 80.00%.

@@            Coverage Diff             @@
##             main     #924      +/-   ##
==========================================
- Coverage   72.71%   72.62%   -0.09%     
==========================================
  Files          28       28              
  Lines        2855     2857       +2     
==========================================
- Hits         2076     2075       -1     
- Misses        683      685       +2     
- Partials       96       97       +1     
Files Changed Coverage Δ
core/pkg/eval/json_evaluator.go 84.69% <80.00%> (-0.93%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@beeme1mr beeme1mr left a comment

Choose a reason for hiding this comment

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

Awesome, thanks! 😃

@toddbaert
Copy link
Member

LGTM!

@toddbaert toddbaert changed the title fix: create new context when adding flagd properties fix: erroneous warning about prop overwrite Sep 19, 2023
@toddbaert toddbaert merged commit 673b76a into open-feature:main Sep 19, 2023
@github-actions github-actions bot mentioned this pull request Sep 19, 2023
@craigpastro craigpastro deleted the fix-extra-log-bug branch September 19, 2023 16:31
toddbaert pushed a commit that referenced this pull request Oct 12, 2023
🤖 I have created a release *beep* *boop*
---


<details><summary>flagd: 0.6.7</summary>

##
[0.6.7](flagd/v0.6.6...flagd/v0.6.7)
(2023-10-12)


### 🐛 Bug Fixes

* **deps:** update module github.com/open-feature/flagd/core to v0.6.6
([#916](#916))
([1f80e4d](1f80e4d))
* **deps:** update module
github.com/open-feature/go-sdk-contrib/providers/flagd to v0.1.17
([#759](#759))
([a2a2c3c](a2a2c3c))
* **deps:** update module github.com/spf13/viper to v1.17.0
([#956](#956))
([31d015d](31d015d))
* **deps:** update module go.uber.org/zap to v1.26.0
([#917](#917))
([e57e206](e57e206))


### 🧹 Chore

* docs rework ([#927](#927))
([27b3193](27b3193))


### 📚 Documentation

* fixed typos and linting issues
([#957](#957))
([0bade57](0bade57))
</details>

<details><summary>flagd-proxy: 0.2.12</summary>

##
[0.2.12](flagd-proxy/v0.2.11...flagd-proxy/v0.2.12)
(2023-10-12)


### 🐛 Bug Fixes

* **deps:** update module github.com/open-feature/flagd/core to v0.6.6
([#916](#916))
([1f80e4d](1f80e4d))
* **deps:** update module github.com/spf13/viper to v1.17.0
([#956](#956))
([31d015d](31d015d))
* **deps:** update module go.uber.org/zap to v1.26.0
([#917](#917))
([e57e206](e57e206))
</details>

<details><summary>core: 0.6.7</summary>

##
[0.6.7](core/v0.6.6...core/v0.6.7)
(2023-10-12)


### 🐛 Bug Fixes

* **deps:** update module github.com/prometheus/client_golang to v1.17.0
([#939](#939))
([9065cba](9065cba))
* **deps:** update module github.com/rs/cors to v1.10.1
([#946](#946))
([1c39862](1c39862))
* **deps:** update module go.uber.org/zap to v1.26.0
([#917](#917))
([e57e206](e57e206))
* **deps:** update module golang.org/x/mod to v0.13.0
([#952](#952))
([be61450](be61450))
* **deps:** update module golang.org/x/sync to v0.4.0
([#949](#949))
([faa24a6](faa24a6))
* **deps:** update module google.golang.org/grpc to v1.58.1
([#915](#915))
([06d95de](06d95de))
* **deps:** update module google.golang.org/grpc to v1.58.2
([#928](#928))
([90f1878](90f1878))
* **deps:** update module google.golang.org/grpc to v1.58.3
([#960](#960))
([fee1558](fee1558))
* **deps:** update opentelemetry-go monorepo
([#943](#943))
([e7cee41](e7cee41))
* erroneous warning about prop overwrite
([#924](#924))
([673b76a](673b76a))


### ✨ New Features

* add `$flagd.timestamp` to json evaluator
([#958](#958))
([a1b04e7](a1b04e7))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@github-actions github-actions bot mentioned this pull request Dec 2, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Warning about overwriting $flagd property when perform bulk evaluation
3 participants