Skip to content
This repository was archived by the owner on Aug 30, 2019. It is now read-only.

config: add 'site' option#485

Merged
gbbr merged 2 commits into
masterfrom
gbbr/site
Oct 11, 2018
Merged

config: add 'site' option#485
gbbr merged 2 commits into
masterfrom
gbbr/site

Conversation

@gbbr

@gbbr gbbr commented Oct 9, 2018

Copy link
Copy Markdown
Contributor

Adds support for a new site top level configuration option, such that:

  • When site is set, https://trace.agent.<site> is used.
  • When both site and apm_dd_url are set, the latter takes precedence and user is notified.
  • When only apm_dd_url is set, that value will be used.

@gbbr
gbbr requested review from olivielpeau and palazzem October 9, 2018 10:05
Comment thread config/merge_yaml.go
c.APIEndpoint = apiEndpointPrefix + yc.Site
}
if yc.TraceAgent.Endpoint != "" {
c.APIEndpoint = yc.TraceAgent.Endpoint

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

Comment thread config/agent_test.go
})
}

func TestSite(t *testing.T) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@olivielpeau can you double check if it respects entirely our specification?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

it does 💯

@olivielpeau olivielpeau left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, just a nit on the log message

Comment thread config/merge_env.go Outdated
Comment thread config/agent_test.go
})
}

func TestSite(t *testing.T) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

it does 💯

@gbbr

gbbr commented Oct 10, 2018

Copy link
Copy Markdown
Contributor Author

@palazzem PTAL

@gbbr gbbr added this to the 6.6.0 milestone Oct 11, 2018
@gbbr
gbbr merged commit 16c60ae into master Oct 11, 2018
@gbbr
gbbr deleted the gbbr/site branch October 11, 2018 09:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants