feat: enterprise edition #1575

mindhash · 2022-09-16T09:46:53Z

This PR introduces:

feature flagging
EE code separation
licensing (for EE)
SAML SSO (for EE)

ee/query-service/app/api/api.go

ee/query-service/app/api/auth.go

Signed-off-by: Prashant Shahi <prashant@signoz.io>

ee/query-service/app/api/api.go

ee/query-service/app/api/auth.go

ee/query-service/app/db/reader.go

Signed-off-by: Prashant Shahi <prashant@signoz.io>

srikanthccv · 2022-10-03T13:55:57Z

pkg/query-service/config/prometheus.yml

@@ -22,4 +22,4 @@ rule_files:
 scrape_configs: []

 remote_read:
-  - url: tcp://localhost:9000/?database=signoz_metrics
+  - url: tcp://stagingapp.signoz.io:9000/?database=signoz_metrics


Suggested change

- url: tcp://stagingapp.signoz.io:9000/?database=signoz_metrics

- url: tcp://localhost:9000/?database=signoz_metrics

I will remove this file from PR.

pkg/query-service/auth/auth.go

srikanthccv · 2022-10-03T14:11:52Z

pkg/query-service/model/errors.go

+type ErrEmailRequired struct{}
+
+func (errEmailRequired ErrEmailRequired) Error() string {
+	return "email is required"
+}


I understand the intent behind this but there is no data associated with these error types. What do you think about defining them with errors.New/fmt.Errorf?

I think we should adopt named error codes as a pattern. This will help with a centralised control on error messages. especially if we consider applying translations (internationalisation) in future.

Most well-known libraries take a named errors approach as well. e.g. AWS sdk

The error structs can evolve contain into internal message (to be written to log) for developers and the error message to be displayed to the end user. At the moment, we just show 'something went wrong'. Printing internal error on web console (developer console) will help with additional context. Having requestId will also help.

In this PR, I could not make all changes at once but wanted to initiate this change atleast for generic and common errors.

I generally find them to be useful and think same pattern doesn't fit well with all types of errors. I think the question boils down what should be named error? For example the os package defines several error with errors.New where the error is simply a message https://github.com/golang/go/blob/efa357ce3c1a8ef0ce94347fef533c87673a598a/src/os/error.go#L16-L24 which are basically taken from here https://github.com/golang/go/blob/2ebe77a2fda1ee9ff6fd9a3e08933ad1ebaea039/src/internal/oserror/errors.go. If there is additional functionality we can't achieve with them then it makes sense to go with named errors.

I find this is more concise.

var ( ErrEmailRequired = errors.New("email is required") ErrPasswordRequired = errors.New("password is required") ErrSignupFailed = errors.New("failed to register user") ErrNoOrgFound = errors.New("no org found") )

I am going to skip this for now. We can discuss more, as I will log a separate issue for named errors. I think if we are looking translations (internationalisation) a struct enables adding language code. Usually language code will come from the current user.

The front-end already takes care of majority of translations but doesn't have control over the error messages.

e.g.
ErrEmailRequired {
Lang: string;
}

func (e *ErrEmailRequired) Error() string {
case e.Lang {
case English.US:
return 'Email is required''
Case Spanish:
....
}

pkg/query-service/auth/auth.go

ee/query-service/saml/request.go

srikanthccv · 2022-10-03T14:34:02Z

ee/query-service/model/errors.go

+type ApiError struct {
+	Typ basemodel.ErrorType
+	Err error
+}
+
+func (a *ApiError) Type() basemodel.ErrorType {
+	return a.Typ
+}
+
+func (a *ApiError) ToError() error {
+	if a != nil {
+		return a.Err
+	}
+	return a.Err
+}


Is this required? I see they are no different from the base model. Do you think these definitions will evolve to be different for ee? I doubt it.

I wanted to reduce references to base code (non-ee) in ee code.

Ideally the BaseApiError(Interface) should be used on both sides passing errors between methods . both EE and OSS can have their own implementations. I couldn't make this change in base (oss) as I wanted to limit the changes in base in this PR.

EE error reporting can evolve differently especially features like sending error reports etc can come in.

EE build changes - Docker files and Makefile

srikanthccv · 2022-10-03T18:27:00Z

pkg/query-service/model/errors.go

+type ErrEmailRequired struct{}
+
+func (errEmailRequired ErrEmailRequired) Error() string {
+	return "email is required"
+}


I generally find them to be useful and think same pattern doesn't fit well with all types of errors. I think the question boils down what should be named error? For example the os package defines several error with errors.New where the error is simply a message https://github.com/golang/go/blob/efa357ce3c1a8ef0ce94347fef533c87673a598a/src/os/error.go#L16-L24 which are basically taken from here https://github.com/golang/go/blob/2ebe77a2fda1ee9ff6fd9a3e08933ad1ebaea039/src/internal/oserror/errors.go. If there is additional functionality we can't achieve with them then it makes sense to go with named errors.

I find this is more concise.

var ( ErrEmailRequired = errors.New("email is required") ErrPasswordRequired = errors.New("password is required") ErrSignupFailed = errors.New("failed to register user") ErrNoOrgFound = errors.New("no org found") )

pkg/query-service/auth/auth.go

ee/query-service/saml/request.go

srikanthccv · 2022-10-03T19:27:18Z

ee/query-service/model/license.go

+	ValidFrom  int64  `json:"ValidFrom"`
+	ValidUntil int64  `json:"ValidUntil"`


Suggested change

ValidFrom int64 `json:"ValidFrom"`

ValidUntil int64 `json:"ValidUntil"`

ValidFrom int64 `json:"validFrom"`

ValidUntil int64 `json:"validUntil"`

srikanthccv · 2022-10-03T19:43:58Z

ee/query-service/license/manager.go

+
+	if err != nil {
+		zap.S().Errorf("failed to activate license", zap.Error(err))
+		return nil, model.BadRequest(err)


model.InternalError would probably more appropriate here.

ee/query-service/license/manager.go

ee/query-service/app/db/reader.go

ee/query-service/app/api/auth.go

fix: move usage reporting to top level ee folder

Signed-off-by: Prashant Shahi <prashant@signoz.io>

chore(ee): 🔧 LD_FLAGS related changes

sonarqubecloud · 2022-10-06T12:48:27Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
18 Code Smells

No Coverage information
3.9% Duplication

srikanthccv

Overall LGTM. Please create an issue for points we decided to continue the discussion.

mindhash added 6 commits September 5, 2022 23:28

feat: added license manager and feature flags

2880dab

chore: deleted unncessary file

f267347

feat: added feature gating mechanics

a91625c

feat: completed org domain api

6bee984

chore: checking in saml auth handler code

fb246b4

chore: merged develop and resolved conflicts

2d8f7c7

mindhash requested a review from ankitnayan September 16, 2022 09:47

mindhash added 22 commits September 20, 2022 18:30

feat: added signup with sso

98ed96b

fix: solved issue with put domain

2957f62

feat: added login support for admins

a9723e2

fix: resolves issue with license activation

ac9fd04

fix: resolved org create issue

a59cf8e

fix: domain creation issue

f8adc58

chore: removed test reference to localhost

594c723

fix: solved an issue with saml auth

3a32fc6

chore: added useful log in license validation routine

29904a5

chore: added dockerfile for ee

cc77ae7

chore: changed dockerfile to dockerfile.ee

710949d

feat: added pem support for certificate

7046693

chore: changed issuer to support active dir setup

24d04aa

chore: added debug messg to resolve registeruser issue

5ac4a1a

fix: unescaped source url

8048889

fix: removed debug message

992056e

chore: moved go.mod to top folder

f9025e7

chore: added ee folder move

7b2d741

Merge branch 'develop' into feat/ee

f5158d6

chore: fixed some mock setups

605a89d

Merge branch 'feat/ee' of github.com:mindhash/signoz into feat/ee

4f55ce2

chore: updated makefile of top folder

6989b9d

mindhash marked this pull request as ready for review October 1, 2022 17:06

mindhash added 5 commits October 3, 2022 11:23

chore: added ee/signoz.db to remove it in git

43515d4

chore: deleted ee/signoz.db

b1a39b2

chore: removed .signoz.db

7873102

chore: removed wrong db file in repo

0fdea3d

chore: added condition to ignore all sqlite db files

cdd21ec

srikanthccv reviewed Oct 3, 2022

View reviewed changes

prashant-shahi added 4 commits October 3, 2022 17:17

chore: 🙈 update gitignore and dockerignore file

a1b85ae

Signed-off-by: Prashant Shahi <prashant@signoz.io>

chore: 🚀 update ee dockerfile

14e63ac

Signed-off-by: Prashant Shahi <prashant@signoz.io>

chore: 🚀 include ee build steps in Makefile

416453e

Signed-off-by: Prashant Shahi <prashant@signoz.io>

ci(build-workflow): 👷 include EE query-service

a3749ff

Signed-off-by: Prashant Shahi <prashant@signoz.io>

srikanthccv reviewed Oct 3, 2022

View reviewed changes

ee/query-service/app/api/api.go Show resolved Hide resolved

ee/query-service/app/api/api.go Show resolved Hide resolved

ee/query-service/app/api/auth.go Show resolved Hide resolved

ee/query-service/app/db/reader.go Show resolved Hide resolved

fix: 🐛 resolve issues of build files

407184d

Signed-off-by: Prashant Shahi <prashant@signoz.io>

srikanthccv reviewed Oct 3, 2022

View reviewed changes

mindhash and others added 4 commits October 3, 2022 20:15

Merge pull request #1 from prashant-shahi/prashant/ee-build

f5a179b

EE build changes - Docker files and Makefile

Merge branch 'develop' into feat/ee

853f110

fix: 🐛 update package name

97a97b4

fix: move usage reporting to top level ee folder

790f049

srikanthccv reviewed Oct 3, 2022

View reviewed changes

mindhash added 5 commits October 4, 2022 13:09

Merge pull request #2 from nityanandagohain/feat/usage-reporting-pr

ddf830a

fix: move usage reporting to top level ee folder

fix: resovled some review comments and issue with license page

ad5a6d9

Merge branch 'feat/ee' of github.com:mindhash/signoz into feat/ee

cc37c5c

chore: restored prom config

a8d6d9d

Merge branch 'develop' into feat/ee

d822ca1

mindhash requested review from srikanthccv and ankitnayan October 6, 2022 12:42

prashant-shahi and others added 2 commits October 6, 2022 18:29

chore(ee): 🔧 LD_FLAGS related changes

9ee3962

Signed-off-by: Prashant Shahi <prashant@signoz.io>

Merge pull request #3 from prashant-shahi/prashant/ld-flags

a05a6fd

chore(ee): 🔧 LD_FLAGS related changes

srikanthccv approved these changes Oct 6, 2022

View reviewed changes

ankitnayan merged commit 9c4521b into SigNoz:develop Oct 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: enterprise edition #1575

feat: enterprise edition #1575

mindhash commented Sep 16, 2022

srikanthccv Oct 3, 2022

mindhash Oct 3, 2022

srikanthccv Oct 3, 2022

mindhash Oct 3, 2022

srikanthccv Oct 3, 2022

mindhash Oct 6, 2022

srikanthccv Oct 3, 2022

mindhash Oct 3, 2022

srikanthccv Oct 3, 2022

srikanthccv Oct 3, 2022

srikanthccv Oct 3, 2022

mindhash Oct 6, 2022

sonarqubecloud bot commented Oct 6, 2022

srikanthccv left a comment

	- url: tcp://stagingapp.signoz.io:9000/?database=signoz_metrics
	- url: tcp://localhost:9000/?database=signoz_metrics

		ValidFrom int64 `json:"ValidFrom"`
		ValidUntil int64 `json:"ValidUntil"`

feat: enterprise edition #1575

feat: enterprise edition #1575

Conversation

mindhash commented Sep 16, 2022

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

sonarqubecloud bot commented Oct 6, 2022

srikanthccv left a comment

Choose a reason for hiding this comment