add featureflag service (Erlang/Elixir) #142

tsloughter · 2022-06-15T20:06:04Z

HTTP interface is on port FEATURE_FLAG_SERVICE_PORT and
the grpc service is run on FEATURE_FLAG_GRPC_SERVICE_PORT.

Creating and updating flags is done through the web interface.
The GRPC service only provides fetching of a flag by name.

Fixes #40

Changes

Please provide a brief description of the changes here.

For significant contributions please make sure you have completed the following items:

Appropriate CHANGELOG.md updated for non-trivial changes
Design discussion issue #

tsloughter · 2022-06-15T20:11:48Z

I was going to remove the create, update, list and delete flag rpc's from the service but I wanted to first check if there was any reason a service that uses feature flags might need to do something besides fetching the flag status? For now those rpc's just return "unimplemented" but could be implemented easy enough, and the creation and updating of the flags must be done through the web interface.

The web pages are just the default generated CRUD pages for a Phoenix project and should be re-styled to match the rest of the demo. I'm going to see if anyone in the Erlang/Elixir SIG wants to help, otherwise I'll get around to it at some point. It took a while to find time to do this much and the majority of it is generated :)

HTTP interface is on port FEATURE_FLAG_SERVICE_PORT and the grpc service is run on FEATURE_FLAG_GRPC_SERVICE_PORT. Creating and updating flags is done through the web interface. The GRPC service only provides fetching of a flag by name.

internal/tools/sanitycheck.py

austinlparker · 2022-06-16T13:50:26Z

I was going to remove the create, update, list and delete flag rpc's from the service but I wanted to first check if there was any reason a service that uses feature flags might need to do something besides fetching the flag status? For now those rpc's just return "unimplemented" but could be implemented easy enough, and the creation and updating of the flags must be done through the web interface.

I can't think of a reason that an existing application might want to modify via RPC. That said, I can see a world where we want to provision flags at runtime/build time... although, in that case it'd make more sense to do so via the DB init script.

julianocosta89 · 2022-06-16T15:14:55Z

@tsloughter I can't build the image to test locally.
1st it failed on step: COPY rel rel complaining that rel didn't exist.

I've played a bit just to try to see if the rel folder was being created, but then the docker build started to fail at the step: mix assets.deploy.
This one was failing because app.js couldn't find import topbar from "../vendor/topbar".

I've stopped the docker build at this step and exec into it to do some digging, and it seems that the vendor folder is not being created in the assets folder.

Do we need any extra steps in the Dockerfile?

tsloughter · 2022-06-16T15:55:55Z

@julianocosta89 ok, I'll try blowing away everything not committed to git and see if it fails for me and will debug from there.

tsloughter · 2022-06-16T15:57:15Z

Woops, yup, rel was not committed, hehe.

tsloughter · 2022-06-16T16:00:16Z

I don't understand why it doesn't show up in git's list of untracked files... That is why I missed it. I recreated it and still doesn't show up.

tsloughter · 2022-06-16T16:04:34Z

Ah. the top level .gitignore has bin/, which is the only dir with files under rel.

tsloughter · 2022-06-16T16:22:44Z

It should work now.

docker-compose.yml

austinlparker

lgtm! builds + runs in codespaces. I went ahead and added an external port binding to the web UI for the flag service to make it more easily accessible -- we should consider putting this behind auth once that's available.

mic-max

Looks good and thanks!!

It does feel like this service is quite a lot of files and maybe has some extra complexity that we can see about simplifying in the future.

mic-max · 2022-06-20T18:50:24Z

internal/tools/sanitycheck.py

@@ -55,7 +55,7 @@ def sanitycheck(pattern, allow_utf8 = False, allow_eol = (CRLF, LF), indent = 1)
                        if c != 32:
                            break
                        spc_count += 1
-                    if not indent or spc_count % indent:
+                    if not indent or (spc_count % indent and os.path.basename(filename) != 'rebar.config'):


Is there a better way to ignore files for the sanitycheck.py script?

mic-max · 2022-06-20T18:52:06Z

src/featureflagservice/.dockerignore

@@ -0,0 +1,45 @@
+# This file excludes paths from the Docker build context.


Note to self: Review this

mic-max · 2022-06-20T18:53:22Z

src/featureflagservice/.gitignore

@@ -0,0 +1,37 @@
+# The directory Mix will write compiled artifacts to.


Add to top-level .gitignore file instead

src/featureflagservice/Dockerfile

mic-max · 2022-06-20T18:59:59Z

src/featureflagservice/README.md

+Then start Postgres with `docker compose`
+
+``` shell
+docker compose up


docker compose up ffs_postgres ?

mic-max · 2022-06-20T19:42:19Z

src/featureflagservice/lib/featureflagservice/mailer.ex

+defmodule Featureflagservice.Mailer do
+  use Swoosh.Mailer, otp_app: :featureflagservice
+end


mic-max · 2022-06-20T22:37:31Z

src/featureflagservice/proto/featureflag.proto

@@ -0,0 +1,70 @@
+// Licensed under the Apache License, Version 2.0 (the "License");


Ideally this file will not be copied/duplicated.

Yup. Issue is simply that the tool used to generate the proto modules has no option for only generating certain services or messages. I plan to remove this and use the top level demo.proto once that is resolved.

The tool used to generate is the normal protocol buffer compiler protoc from Google right?
Are they adding that functionality?! That's cool and we'd likely make use in other services here too!

Ah, na. Though I assumed protoc already supported that, haha, I've run into this enough I'm surprised. It uses https://github.com/tomas-abrahamsson/gpb/ through the plugin for grpc generation, https://github.com/tsloughter/grpcbox_plugin/

Ideally I could get gpb to also only create the encode/decode for the messages the services I need depend on, but I can at least pretty quickly have the grpc plugin only generate the service modules for the featureflag service.

mic-max · 2022-06-20T22:38:44Z

src/featureflagservice/rel/overlays/bin/migrate

@@ -0,0 +1,3 @@
+#!/bin/sh


What are these scripts in the rel/overlays directory?

These scripts are what are used to start the Release. See the command at the end of the dockerfile. Which I now see uses CMD instead of ENTRYPOINT for some reason... may change that.

Ah, nm, I bet they use CMD because the wrapper script always runs start, so no use doing a separate entrypoint and cmd like I am used to in Erlang Releases.

src/featureflagservice/src/ffs_service_bhvr.erl

mic-max · 2022-06-21T00:06:50Z

src/featureflagservice/src/ffs_service_client.erl

+%% @end
+%%%-------------------------------------------------------------------
+
+%% this module was generated and should not be modified manually


tsloughter · 2022-06-21T11:49:11Z

@mic-max yup, there is a lot of extra stuff. I'm hoping to get someone who actually works with the Phoenix framework to take on removing what is unncessary (like the swoosh mail module you point out, the topbar.js, etc). I can certainly do it, but I'll miss stuff, or be spending time on trial and error removal :). I'll try again to get someone to work on this and if I don't get anyone this week I'll start whittling away.

I'll comment individually on the other questions you raised.

* add featureflag service (Erlang/Elixir) HTTP interface is on port FEATURE_FLAG_SERVICE_PORT and the grpc service is run on FEATURE_FLAG_GRPC_SERVICE_PORT. Creating and updating flags is done through the web interface. The GRPC service only provides fetching of a flag by name. * ignore space indent count if file is rebar.config * featureflagservice: add release directory to git * featureflagservice: include vendored js in git repo * Update docker-compose.yml Co-authored-by: Austin Parker <[email protected]>

tsloughter requested a review from a team June 15, 2022 20:06

tsloughter force-pushed the featureflagservice branch 2 times, most recently from f759a30 to be6ec3b Compare June 15, 2022 20:19

add featureflag service (Erlang/Elixir)

7e9b57d

HTTP interface is on port FEATURE_FLAG_SERVICE_PORT and the grpc service is run on FEATURE_FLAG_GRPC_SERVICE_PORT. Creating and updating flags is done through the web interface. The GRPC service only provides fetching of a flag by name.

tsloughter force-pushed the featureflagservice branch from be6ec3b to 7e9b57d Compare June 15, 2022 20:20

ignore space indent count if file is rebar.config

2b53c9d

reyang reviewed Jun 15, 2022

View reviewed changes

internal/tools/sanitycheck.py Show resolved Hide resolved

tsloughter added 2 commits June 16, 2022 10:06

featureflagservice: add release directory to git

850229c

featureflagservice: include vendored js in git repo

62493be

austinlparker reviewed Jun 17, 2022

View reviewed changes

docker-compose.yml Outdated Show resolved Hide resolved

Update docker-compose.yml

c261067

austinlparker approved these changes Jun 17, 2022

View reviewed changes

austinlparker merged commit 5ec9223 into open-telemetry:main Jun 17, 2022

mic-max reviewed Jun 21, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add featureflag service (Erlang/Elixir) #142

add featureflag service (Erlang/Elixir) #142

tsloughter commented Jun 15, 2022

tsloughter commented Jun 15, 2022

austinlparker commented Jun 16, 2022

julianocosta89 commented Jun 16, 2022

tsloughter commented Jun 16, 2022

tsloughter commented Jun 16, 2022

tsloughter commented Jun 16, 2022

tsloughter commented Jun 16, 2022

tsloughter commented Jun 16, 2022

austinlparker left a comment

mic-max left a comment

mic-max Jun 20, 2022

mic-max Jun 20, 2022

mic-max Jun 20, 2022

mic-max Jun 20, 2022

mic-max Jun 20, 2022

mic-max Jun 20, 2022

tsloughter Jun 21, 2022

mic-max Jun 21, 2022

tsloughter Jun 21, 2022

mic-max Jun 20, 2022

tsloughter Jun 21, 2022

tsloughter Jun 21, 2022

mic-max Jun 21, 2022

tsloughter commented Jun 21, 2022

		@@ -0,0 +1,45 @@
		# This file excludes paths from the Docker build context.

		@@ -0,0 +1,37 @@
		# The directory Mix will write compiled artifacts to.

		@@ -0,0 +1,70 @@
		// Licensed under the Apache License, Version 2.0 (the "License");

add featureflag service (Erlang/Elixir) #142

add featureflag service (Erlang/Elixir) #142

Conversation

tsloughter commented Jun 15, 2022

Changes

tsloughter commented Jun 15, 2022

austinlparker commented Jun 16, 2022

julianocosta89 commented Jun 16, 2022

tsloughter commented Jun 16, 2022

tsloughter commented Jun 16, 2022

tsloughter commented Jun 16, 2022

tsloughter commented Jun 16, 2022

tsloughter commented Jun 16, 2022

austinlparker left a comment

Choose a reason for hiding this comment

mic-max left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tsloughter commented Jun 21, 2022