microbit-foundation / cctd-ml-machine

GitHub Location for Aarhus University's Center for Computational Thinking and Design's ML-Machine application.
https://ml-machine.org/
MIT License
5 stars 10 forks source link

Save the appinsights connectionString in an environment variable #489

Open Karlo-Emilo opened 4 months ago

Karlo-Emilo commented 4 months ago

so we set connectionString to process.env.APPINSIGHTS_CONNECT and define the environment variable in the Azure webapp environment.

By doing this, we can set the connectionString in the specific webapp instance and ensure that we only track data for that instance.

If the environment variable does not exist (e.g. in dev environments or test instances), appInsights should not be loaded.

Karlo-Emilo commented 4 months ago

I have made the environment variables in Azure for ml.amchine.org and main.ml-machine.org.

Environment variable name: APPINSIGHTS_CONNECT (so it can be accessed through process.env.APPINSIGHTS_CONNECT)

We can just test it on the main branch.

Karlo-Emilo commented 4 months ago

I realized the environment variable is, of course, not available client-side. So we will have to pass it on manually. How could we do that, @r59q ?

It is important to note that the connection string should be set in the running instance (e.g., an Azure webapp instance), not in the source code. So it will have to be fetched from process.env

r59q commented 4 months ago

process.env is a node function. Node is the runtime that is being run on the server (maybe)

here is an example of a server. It has access to the node runtime. If we must access the environment variables at runtime, then we could add an endpoint. I.e

// Serve the ml-machine website files
app.get('/', async (req, res) => {
    res.sendFile(path.join(__dirname, 'index.html'));
});

// REST endpoint to serve the appinsights endpoint
app.get('/appinsights', async (req, res) => {
    res.send(process.env.APPINSIGHTS_CONNECT)
});

app.listen(8080, () => {
    console.log("Server successfully running on port 8080");
});

Then from the code we could do something like

const insightsConnectionString = await fetch("/appinsights")

export const appInsights = new ApplicationInsights({
  config: {
    connectionString: insightsConnectionString,
  },
});

This is just an example for explanation. I actually have no idea what server(example is just an express server) is being used, nor what capabilities we have using azure. If for instance the files were served using NGINX, then we would have to write a location block in it's configuration

http {
    server {
        listen       80;
        location / {
            access_log off;
            return 200 '${APPINSIGHTS_CONNECT}';
            add_header Content-Type text/plain;
        }
    }
}

Another approach

I think we could use the environment variables at build time (much like the prepEnv.js script) to inject the environment variable at build-time

currently we use it like

node prepEnv.js branded

We could add the environment variable

node prepEnv.js branded $APPINSIGHTS_CONNECT

then in the script we create a file containing the connection string before the build process is started

const args = process.argv;
var ob = { insightsString : args[3] };

fs.writeFile("./insights.json", JSON.stringify(ob, null, 4), (err) => {
    if (err) {  console.error(err);  return; };
    console.log("File has been created");
});

finally we use it

const json = JSON.parse(
  await readFile(
    new URL('./insights.json', import.meta.url)
  )
);

export const appInsights = new ApplicationInsights({
  config: {
    connectionString: json.insightsString,
  },
});

However do note, that the environment variable has to be available to the agent that builds the application, i'm not sure if it's the github workflow runner or an azure instance?

Karlo-Emilo commented 4 months ago

It looks like we are exposing all environment variables client-side: https://github.com/microbit-foundation/cctd-ml-machine/blob/a90b7f1f1351d42f8958fc4d932b4201f283ef8a/vite.config.ts#L26 (which may be a dangerous practice)

So, the environment variable may already be available to the client side.

Otherwise, we could write the environment variable in a .env file in the build process.

import * as fs from 'fs';

const envText = "APPINSIGHTS_CONNECT=" + process.env.APPINSIGHTS_CONNECT;

fs.writeFileSync(".env", envText);

This would make it available with our current settings, where we expose all environment variables.

Karlo-Emilo commented 4 months ago

It looks like we are exposing all environment variables client-side:

https://github.com/microbit-foundation/cctd-ml-machine/blob/a90b7f1f1351d42f8958fc4d932b4201f283ef8a/vite.config.ts#L26

(which may be a dangerous practice) So, the environment variable may already be available to the client side.

Otherwise, we could write the environment variable in a .env file in the build process.

import * as fs from 'fs';

const envText = "APPINSIGHTS_CONNECT=" + process.env.APPINSIGHTS_CONNECT;

fs.writeFileSync(".env", envText);

This would make it available with our current settings, where we expose all environment variables.

I realize that there may be issues with permissions to write in files.

Karlo-Emilo commented 3 months ago

Update: it does not work when deployed to Azure.

The environment variable is set: image

The app is deployed (after the environment variable is set): https://zealous-bay-0c5f48303-503.westeurope.2.azurestaticapps.net/

But is the environment variable is not available: image

The next step is to print the key during the build process to check whether it is available.

Karlo-Emilo commented 3 months ago

Okay, the build process may happen on GitHub, not Azure. I will try with Github environments instead. (https://docs.github.com/en/actions/deployment/targeting-different-environments/using-environments-for-deployment)

This could also improve the build process in general