Skip to content

feat: add $flagd.timestamp to json evaluator#958

Merged
toddbaert merged 4 commits intoopen-feature:mainfrom
craigpastro:flagd-properties
Oct 11, 2023
Merged

feat: add $flagd.timestamp to json evaluator#958
toddbaert merged 4 commits intoopen-feature:mainfrom
craigpastro:flagd-properties

Conversation

@craigpastro
Copy link
Copy Markdown
Member

@craigpastro craigpastro commented Oct 6, 2023

This PR

Adds "timestamp" to the json evaluation context.

Related Issues

Related to #851. I am not sure we want to say that it closes the issue though.

feature: add timestamp to json evaluator
7911878 
Signed-off-by: Craig Pastro <pastro.craig@gmail.com>
@craigpastro craigpastro requested a review from a team October 6, 2023 21:27
@netlify
Copy link
Copy Markdown

netlify bot commented Oct 6, 2023 •
edited
Loading

✅ Deploy Preview for polite-licorice-3db33c canceled.

Name Link
🔨 Latest commit c3a3cf4
🔍 Latest deploy log https://app.netlify.com/sites/polite-licorice-3db33c/deploys/6526a5bd7f14b40008aa1898

@craigpastro craigpastro changed the title feature: add timestamp to json evaluator feat: add timestamp to json evaluator Oct 6, 2023
@codecov
Copy link
Copy Markdown

codecov bot commented Oct 6, 2023 •
edited
Loading

Codecov Report

Merging #958 (c3a3cf4) into main (fee1558) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #958   +/-   ##
=======================================
  Coverage   72.62%   72.63%           
=======================================
  Files          28       28           
  Lines        2857     2858    +1     
=======================================
+ Hits         2075     2076    +1     
  Misses        685      685           
  Partials       97       97
Files Coverage Δ
core/pkg/eval/json_evaluator.go 84.74% <100.00%> (+0.05%) ⬆️

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

toddbaert
toddbaert reviewed Oct 9, 2023
View reviewed changes
Use unix timestamp
96d4e47 
Signed-off-by: Craig Pastro <pastro.craig@gmail.com>
@toddbaert toddbaert self-requested a review October 10, 2023 16:48
toddbaert
toddbaert approved these changes Oct 10, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@toddbaert toddbaert left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Makes sense to me! Thanks!

@Kavindu-Dodan @bacherfl @beeme1mr @thisthat what do you think about this implementation? And about my points here?

I think this is a nice feature, because it can allow for a really simple "timed rollout", especially when combined with the fractional operation.

@toddbaert toddbaert changed the title feat: add timestamp to json evaluator feat: add $flagd.timestamp to json evaluator Oct 10, 2023
@Kavindu-Dodan
Copy link
Copy Markdown
Contributor

Kavindu-Dodan commented Oct 10, 2023

Makes sense to me! Thanks!

@Kavindu-Dodan @bacherfl @beeme1mr @thisthat what do you think about this implementation? And about my points here?

I think this is a nice feature, because it can allow for a really simple "timed rollout", especially when combined with the fractional operation.

Timed rollouts are an interesting feature. Besides, this can be used combined with other json logic operators to enable flag in time range.

beeme1mr
beeme1mr approved these changes Oct 11, 2023
View reviewed changes
Copy link
Copy Markdown
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.

Love it, thanks!

As a follow up, can you update the following docs to include information on these flagd properties?

In the flag definitions file, adding a section right before Shared evaluators makes sense. It can include a short description that explains that these properties are automagically included in the context. Below that could be a table listing the properties, the version they were introduced, and a short description.

The in-process spec should also include a section that lists flagd properties as a requirement and what their expected behavior is.

@toddbaert
Copy link
Copy Markdown
Member

toddbaert commented Oct 11, 2023

Love it, thanks!

As a follow up, can you update the following docs to include information on these flagd properties?

In the flag definitions file, adding a section right before Shared evaluators makes sense. It can include a short description that explains that these properties are automagically included in the context. Below that could be a table listing the properties, the version they were introduced, and a short description.

The in-process spec should also include a section that lists flagd properties as a requirement and what their expected behavior is.

Agreed on all counts. I didn't want to do this until everyone agreed on this approach, but since it's clear we all do I'll do documentation next (or create an issue for it).

@toddbaert toddbaert mentioned this pull request Oct 11, 2023
Closed
@toddbaert toddbaert merged commit a1b04e7 into open-feature:main Oct 11, 2023
@github-actions github-actions bot mentioned this pull request Oct 11, 2023
Merged
@craigpastro craigpastro deleted the flagd-properties branch October 11, 2023 17:05
toddbaert pushed a commit that referenced this pull request Oct 12, 2023
@github-actions
chore: release main (#919)
3dd6929 
🤖 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
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@beeme1mr beeme1mr beeme1mr approved these changes

@thisthat thisthat thisthat approved these changes

@bacherfl bacherfl bacherfl approved these changes

@toddbaert toddbaert toddbaert approved these changes

+1 more reviewer

@Kavindu-Dodan Kavindu-Dodan Kavindu-Dodan approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@craigpastro @Kavindu-Dodan @toddbaert @beeme1mr @thisthat @bacherfl