Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fix semantics and rename http flag #2122

Merged
merged 3 commits into from
Nov 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/workflows/proto-pr.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ jobs:

- uses: bufbuild/buf-setup-action@3e33fea8596569a62fcaa33eb3880b6ac7098941 # tag=v1.9.0

- name: version
run: buf --version

- name: Format
run: buf format --diff --exit-code

Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/proto-push.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ jobs:

- uses: bufbuild/buf-setup-action@3e33fea8596569a62fcaa33eb3880b6ac7098941 # tag=v1.9.0

- name: version
run: buf --version

- name: Format
run: buf format --diff --exit-code

Expand Down
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,8 @@ Flags:
to a remote gRPC endpoint. All runs all
components.
--log-level="info" log level.
--port=":7070" Port string for server
--http-address=":7070" Address to bind HTTP server to.
--port="" (DEPRECATED) Use http-address instead.
--cors-allowed-origins=CORS-ALLOWED-ORIGINS,...
Allowed CORS origins.
--otlp-address=STRING OpenTelemetry collector address to send
Expand Down
2 changes: 1 addition & 1 deletion deploy/dev.jsonnet
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
function(agentVersion='v0.4.1', separateUI=true)
function(agentVersion='v0.10.0-rc.0', separateUI=true)
local ns = {
apiVersion: 'v1',
kind: 'Namespace',
Expand Down
2 changes: 2 additions & 0 deletions deploy/lib/parca/parca.libsonnet
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,8 @@ function(params) {
args:
[
'/parca',
// http-address optionally specifies the TCP address for the server to listen on, in the form "host:port".
'--http-address=' + ':' + prc.config.port,
'--config-path=/etc/parca/parca.yaml',
'--log-level=' + prc.config.logLevel,
] +
Expand Down
19 changes: 13 additions & 6 deletions pkg/parca/parca.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,12 @@ const (
)

type Flags struct {
ConfigPath string `default:"parca.yaml" help:"Path to config file."`
Mode string `default:"all" enum:"all,scraper-only" help:"Scraper only runs a scraper that sends to a remote gRPC endpoint. All runs all components."`
LogLevel string `default:"info" enum:"error,warn,info,debug" help:"log level."`
Port string `default:":7070" help:"Port string for server"`
ConfigPath string `default:"parca.yaml" help:"Path to config file."`
Mode string `default:"all" enum:"all,scraper-only" help:"Scraper only runs a scraper that sends to a remote gRPC endpoint. All runs all components."`
LogLevel string `default:"info" enum:"error,warn,info,debug" help:"log level."`
HTTPAddress string `default:":7070" help:"Address to bind HTTP server to."`
Port string `default:"" help:"(DEPRECATED) Use http-address instead."`

CORSAllowedOrigins []string `help:"Allowed CORS origins."`
OTLPAddress string `help:"OpenTelemetry collector address to send traces to."`
Version bool `help:"Show application version."`
Expand Down Expand Up @@ -133,6 +135,11 @@ func Run(ctx context.Context, logger log.Logger, reg *prometheus.Registry, flags
defer closer()
}

if flags.Port != "" {
level.Warn(logger).Log("msg", "flag --port is deprecated, use --http-address instead")
Copy link
Member

Choose a reason for hiding this comment

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

We broke a bunch of flags in this release I think adding one more doesn’t make a difference

Copy link
Member

Choose a reason for hiding this comment

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

Agreed.

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

Copy link
Member Author

@kakkoyun kakkoyun Nov 18, 2022

Choose a reason for hiding this comment

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

I'd done it but then reverted; it broke the snap. I think this is one of the oldest flags, so let's not ruin the backward compatibility on this one.

flags.HTTPAddress = flags.Port
}

cfg, err := config.LoadFile(flags.ConfigPath)
if err != nil {
level.Error(logger).Log("msg", "failed to read config", "path", flags.ConfigPath)
Expand Down Expand Up @@ -409,7 +416,7 @@ func Run(ctx context.Context, logger log.Logger, reg *prometheus.Registry, flags
return parcaserver.ListenAndServe(
ctx,
logger,
flags.Port,
flags.HTTPAddress,
flags.CORSAllowedOrigins,
flags.PathPrefix,
server.RegisterableFunc(func(ctx context.Context, srv *grpc.Server, mux *runtime.ServeMux, endpoint string, opts []grpc.DialOption) error {
Expand Down Expand Up @@ -603,7 +610,7 @@ func runScraper(
return parcaserver.ListenAndServe(
serveCtx,
logger,
flags.Port,
flags.HTTPAddress,
flags.CORSAllowedOrigins,
flags.PathPrefix,
server.RegisterableFunc(func(ctx context.Context, srv *grpc.Server, mux *runtime.ServeMux, endpoint string, opts []grpc.DialOption) error {
Expand Down
8 changes: 4 additions & 4 deletions pkg/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,8 @@ func NewServer(reg *prometheus.Registry, version string) *Server {
}

// ListenAndServe starts the http grpc gateway server.
func (s *Server) ListenAndServe(ctx context.Context, logger log.Logger, port string, allowedCORSOrigins []string, pathPrefix string, registerables ...Registerable) error {
level.Info(logger).Log("msg", "starting server", "addr", port)
func (s *Server) ListenAndServe(ctx context.Context, logger log.Logger, addr string, allowedCORSOrigins []string, pathPrefix string, registerables ...Registerable) error {
level.Info(logger).Log("msg", "starting server", "addr", addr)
logLevel := "ERROR"

logOpts := []grpc_logging.Option{
Expand Down Expand Up @@ -135,7 +135,7 @@ func (s *Server) ListenAndServe(ctx context.Context, logger log.Logger, port str

grpcWebMux := runtime.NewServeMux()
for _, r := range registerables {
if err := r.Register(ctx, srv, grpcWebMux, port, opts); err != nil {
if err := r.Register(ctx, srv, grpcWebMux, addr, opts); err != nil {
return err
}
}
Expand Down Expand Up @@ -176,7 +176,7 @@ func (s *Server) ListenAndServe(ctx context.Context, logger log.Logger, port str
}

s.Server = http.Server{
Addr: port,
Addr: addr,
Handler: grpcHandlerFunc(
srv,
fallbackNotFound(internalMux, uiHandler),
Expand Down
23 changes: 11 additions & 12 deletions ui/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ This is a [Create React App](https://create-react-app.dev/) project that utilize

The React app requires an environment variable for the API endpoint so as to talk to the Parca backend. Create a file named `.env.local` in `packages/app/web/` to add the environment variable for the API endpoint.

```
```shell
REACT_APP_PUBLIC_API_ENDPOINT=http://localhost:7070
```

Expand Down Expand Up @@ -46,8 +46,7 @@ make ui/build # yarn install && yarn build

We embed the artifacts (the production build and its static assets) into the final binary distribution.
See https://pkg.go.dev/embed for further details.

Run following to build the `parca` binary with embedded assets.
Run the following to build the `parca` binary with embedded assets.

```shell
make build
Expand All @@ -63,7 +62,7 @@ You can set up a cluster and all else you need by simply running:
make dev/up
```

For a simple local development setup we use [Tilt](https://tilt.dev).
For a simple local development setup, we use [Tilt](https://tilt.dev).

```shell
tilt up
Expand All @@ -73,33 +72,33 @@ tilt up

We have a feature flag system that allows you to enable or disable features for a user browser. It's a naive implementation based on browser local storage.

#### Usage:
### Usage

```
```js
import useUIFeatureFlag from '@parca/functions/useUIFeatureFlag';

const Header = () => {
const isGreetingEnabled = useUIFeatureFlag('greeting');
return (
<div>
<img src="/logo.png" alt="Logo"/>
<img src="/logo.png" alt="Logo" />
{isGreetingEnabled ? <h1>Hello!!!</h1> : null}
</div>
);
}
};
```

For easy modification of the flag states, we added a two utility query params that can be used to control the feature flag state: `enable-ui-flag` and `disable-ui-flag`.
For easy modification of the flag states, we added two utility query params that can be used to control the feature flag state: `enable-ui-flag` and `disable-ui-flag`.

For example, if you want to enable the greeting feature for a browser, you can load the following URL:

```
```text
http://localhost:3000/?enable-ui-flag=greeting
```

Like wise, if you would like to disable the greeting feature for a browser, you can load the following URL:
Likewise, if you would like to disable the greeting feature for a browser, you can load the following URL:

```
```text
http://localhost:3000/?disable-ui-flag=greeting
```

Expand Down
1 change: 0 additions & 1 deletion ui/ui.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import "embed"

//nolint:typecheck
//go:embed packages/app/web/build

var FS embed.FS

// NOTICE: Static HTML export of a Next.js app contains several files prefixed with _,
Expand Down