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

Update sm_url documentation #571

Merged
merged 2 commits into from
Aug 23, 2022
Merged

Update sm_url documentation #571

merged 2 commits into from
Aug 23, 2022

Conversation

mem
Copy link
Contributor

@mem mem commented Jul 28, 2022

Clarify that the value should be the same everywhere and specify where
to find it.

Signed-off-by: Marcelo E. Magallon marcelo.magallon@grafana.com

Clarify that the value _should_ be the same everywhere and specify where
to find it.

Signed-off-by: Marcelo E. Magallon <marcelo.magallon@grafana.com>
@mem mem requested a review from a team as a code owner July 28, 2022 22:39
docs/index.md Outdated
Comment on lines 164 to 165
Also, if the `sm_url` value is specified for one provider instance, it's almost
certain that the same value should be specified for all provider instances.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not entirely sure that I understand what you mean with this and why is that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for looking at this.

I wasn't sure how to word this, so it's not a surprise that it's not clear.

If you look at the docs here you'll see that we tell users to instantiate the provider twice:

// Step 1: Create a stack
provider "grafana" {
  alias         = "cloud"
  cloud_api_key = "<my-api-key>"
}

...

// Step 3: Interact with Synthetic Monitoring
provider "grafana" {
  alias           = "sm"
  sm_access_token = grafana_synthetic_monitoring_installation.sm_stack.sm_access_token
}
...

the reason for this is that the first provider is used to create the Grafana stack and the second provider is used to interact with the Synthetic Monitoring API.

While it might be obvious that the second provider needs the sm_url setting, the first one also needs it because it's the one used to create the grafana_synthetic_monitoring_installation resource.

It could well be that it's not necessary to use two instances of the provider, because as far as I can see, the access token will be set on the SM installation resource after it's created.

But this is what the example is telling users to do...

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I see what you mean now. 🤔 let me think a bit more about it, I believe your updates are correct… but I'd rather have a clearer overall documentation in the first place.

Copy link
Contributor Author

@mem mem Jul 29, 2022

Choose a reason for hiding this comment

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

I think I see the issue...

The token is set on the installation resource after it's created. The problem is that you need to set it as an attribute in the provider, so you need one provider to obtain the access token and a second provider to act on the SM resources. So you need to set the URL in the first provider so that the installation works and then you need to set the URL in the second provider so that the resources are created in the correct location. If you don't, you should see errors because the access token in the first provider won't be valid for the second provider.

So that's what I meant: you have to make sure that you are talking to the same SM API server in all the providers that deal with SM resources (unless you know exactly what you are doing, of course, which is the reason I didn't want to assert that you MUST set the same URL in all the providers).

Copy link
Contributor

@mdb mdb Aug 4, 2022

Choose a reason for hiding this comment

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

@mem With that in mind, would a phrasing such as the following satisfy your concern?

Note that when a Terraform configuration contains multiple provider instances managing SM resources associated with the same Grafana stack, specifying an explicit `sm_url` set to the same value for each provider ensures all providers interact with the same SM API.

Copy link
Contributor

Choose a reason for hiding this comment

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

@mdb yes, that reads better, thank you!

Signed-off-by: Mike Ball <mikedball@gmail.com>

Signed-off-by: Mike Ball <mikedball@gmail.com>
@julienduchesne julienduchesne merged commit 1dd3efe into master Aug 23, 2022
@julienduchesne julienduchesne deleted the update_sm_url_docs branch August 23, 2022 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants